Message ID | 034f56289398fd3198b05b0a5a9b0147d64e92ba.1699493581.git.tristan@cott.am |
---|---|
State | New |
Headers | show |
Series | [bug#67009] build: minetest-build-system: Improve white space handling in mod name field. | expand |
Am Donnerstag, dem 09.11.2023 um 11:43 +0100 schrieb Tristan Cottam: > On Thursday, November 9th, 2023 at 7:06 AM, Liliana Marie Prikler > <liliana.prikler@gmail.com> wrote: > > > > The new regexp is missing a terminator ($). Should we add > > > [[:space:]]$ at the end? > > > > Ahem, [[:space:]]*$ of course. > > I didn't include [[:space:]]*$ in case someone decided to append a > comment to the same line. However, since Minetest doesn't officially > support comments in mod.conf, I think it's safe to include > [[:space:]]*$ for clarity. Note: comments that aren't for the commit itself should go below the dashed (---) line. > * guix/build/minetest-build-system.scm (name-regexp): Improve white > space handling. > > Change-Id: I95f4c201724991a10efba5c859bfef99779ea495 > --- > guix/build/minetest-build-system.scm | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/guix/build/minetest-build-system.scm > b/guix/build/minetest-build-system.scm > index 4a7a87ab83..cfeca7d18f 100644 > --- a/guix/build/minetest-build-system.scm > +++ b/guix/build/minetest-build-system.scm > @@ -126,7 +126,7 @@ (define* (minimise-png #:key inputs native-inputs > #:allow-other-keys) > (/ total-old-size (expt 1024 2)) > (/ total-new-size (expt 1024 2))))))) > > -(define name-regexp (make-regexp "^name[ ]*=(.+)$")) > +(define name-regexp (make-regexp > "^name[[:space:]]*=[[:space:]]*([[:graph:]]+)[[:space:]]*$")) LGTM, but I'd do a line break + indentation before make-regexp. Applied locally with exactly that change; will push unless there's other comments in the next few days. Cheers
Am Donnerstag, dem 09.11.2023 um 13:18 +0100 schrieb Tristan Cottam: > Fixes certain Minetest mods being stored with a terminating > carriage return in their directory base name. Instead of "certain", you might want to name those mods ;) > * guix/build/minetest-build-system.scm (name-regexp): Improve white > space handling. Perhaps describe the actual effect instead of just saying "improve". That is: "Only match graphical characters in the name sub-match." > Change-Id: I95f4c201724991a10efba5c859bfef99779ea495 > --- > > I added the relevant line break and indentation. Can you confirm this > patch is formatted properly this time? Yep, it looks properly formatted. Cheers
The only mods I encountered with this problem aren't packaged upstream yet, I only have them in my own channel. Should I contribute them first in order to refer to them by package name?
Am Freitag, dem 10.11.2023 um 00:59 +0000 schrieb Tristan Cottam: > The only mods I encountered with this problem aren't packaged > upstream yet, I only have them in my own channel. Should I contribute > them first in order to refer to them by package name? In that case it would be helpful to follow this improvement up with one of these mods :) As long as we're talking free software, upstreaming some minetest mods shouldn't be too difficult. Cheers
diff --git a/guix/build/minetest-build-system.scm b/guix/build/minetest-build-system.scm index 4a7a87ab83..8233d56aca 100644 --- a/guix/build/minetest-build-system.scm +++ b/guix/build/minetest-build-system.scm @@ -126,7 +126,7 @@ (define* (minimise-png #:key inputs native-inputs #:allow-other-keys) (/ total-old-size (expt 1024 2)) (/ total-new-size (expt 1024 2))))))) -(define name-regexp (make-regexp "^name[ ]*=(.+)$")) +(define name-regexp (make-regexp "^name[[:space:]]*=[[:space:]]*([[:graph:]]+)")) (define* (read-mod-name mod.conf #:optional not-found) "Read the name of a mod from MOD.CONF. If MOD.CONF