[pacman-dev] Case sensitive options in pacman.conf
Currently, every options in pacman.conf apparently has a fixed case : RootDir = / DBPath = /var/lib/pacman/ CacheDir = /var/cache/pacman/pkg/ LogFile = /var/log/pacman.log HoldPkg = pacman glibc But in fact, pacman does an insensitive comparisons when parsing the file. } else if(!strcmp(key, "CACHEDIR")) { with key = strtoupper("CacheDir") So you can put cAcHeDiR if you like.. Then we found out this insensitive comparison didn't work in some locales, like tr_TR one, where for example, upper(i) != I To work around this, we also added a sensitive comparison : } else if(strcmp(origkey, "CacheDir") == 0 || strcmp(key, "CACHEDIR") == 0) { http://projects.archlinux.org/git/?p=pacman.git;a=commitdiff;h=1cd7567ff8af4... So in fact, in tr_TR locale, only case sensitive options will really work (eg cAcHeDiR won't work). Why do we make a special case for this locale? I think that having to do every single comparisons twice during the config file parsing is really ugly, and more error prone when adding new options. So I would vote for only allowing case sensitive options in pacman.conf, to treat all locales equally, and cleaning the code :) If that's not acceptable, what about adding a new function, to do something like : } else if(compare(origkey, "CacheDir", "CACHEDIR") == 0) { That still isn't ideal but would at least reduce the duplication a bit.
2008/3/11, Xavier
Currently, every options in pacman.conf apparently has a fixed case : RootDir = / DBPath = /var/lib/pacman/ CacheDir = /var/cache/pacman/pkg/ LogFile = /var/log/pacman.log HoldPkg = pacman glibc
But in fact, pacman does an insensitive comparisons when parsing the file. } else if(!strcmp(key, "CACHEDIR")) { with key = strtoupper("CacheDir")
So you can put cAcHeDiR if you like..
Then we found out this insensitive comparison didn't work in some locales, like tr_TR one, where for example, upper(i) != I To work around this, we also added a sensitive comparison : } else if(strcmp(origkey, "CacheDir") == 0 || strcmp(key, "CACHEDIR") == 0) { http://projects.archlinux.org/git/?p=pacman.git;a=commitdiff;h=1cd7567ff8af4...
So in fact, in tr_TR locale, only case sensitive options will really work (eg cAcHeDiR won't work). Why do we make a special case for this locale? I think that having to do every single comparisons twice during the config file parsing is really ugly, and more error prone when adding new options.
So I would vote for only allowing case sensitive options in pacman.conf, to treat all locales equally, and cleaning the code :)
If that's not acceptable, what about adding a new function, to do something like : } else if(compare(origkey, "CacheDir", "CACHEDIR") == 0) {
That still isn't ideal but would at least reduce the duplication a bit.
"only case sensitive" is ok for me, I'm not that lazy to use Shift. :-P -- Roman Kyrylych (Роман Кирилич)
Currently, every options in pacman.conf apparently has a fixed case : RootDir = / DBPath = /var/lib/pacman/ CacheDir = /var/cache/pacman/pkg/ LogFile = /var/log/pacman.log HoldPkg = pacman glibc
But in fact, pacman does an insensitive comparisons when parsing the file. } else if(!strcmp(key, "CACHEDIR")) { with key = strtoupper("CacheDir")
So you can put cAcHeDiR if you like..
Then we found out this insensitive comparison didn't work in some locales, like tr_TR one, where for example, upper(i) != I To work around this, we also added a sensitive comparison : } else if(strcmp(origkey, "CacheDir") == 0 || strcmp(key, "CACHEDIR") ==
2008/3/11, Xavier
: 0) {
http://projects.archlinux.org/git/?p=pacman.git;a=commitdiff;h=1cd7567ff8af4...
So in fact, in tr_TR locale, only case sensitive options will really work
(eg
cAcHeDiR won't work). Why do we make a special case for this locale? I think that having to do every single comparisons twice during the config file parsing is really ugly, and more error prone when adding new options.
So I would vote for only allowing case sensitive options in pacman.conf, to treat all locales equally, and cleaning the code :)
If that's not acceptable, what about adding a new function, to do something like : } else if(compare(origkey, "CacheDir", "CACHEDIR") == 0) {
That still isn't ideal but would at least reduce the duplication a bit.
"only case sensitive" is ok for me, I'm not that lazy to use Shift. :-P +1
---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
On Tue, Mar 11, 2008 at 10:10 AM, Xavier
So I would vote for only allowing case sensitive options in pacman.conf, to treat all locales equally, and cleaning the code :)
I originally brought this up to Dan when I made the tr_TR change. I was for case-sensitive compares, but I think Dan had a bit of an issue with it. Dan, do you remember what it was?
On Tue, Mar 11, 2008 at 11:14:17AM -0500, Aaron Griffin wrote:
On Tue, Mar 11, 2008 at 10:10 AM, Xavier
wrote: So I would vote for only allowing case sensitive options in pacman.conf, to treat all locales equally, and cleaning the code :)
I originally brought this up to Dan when I made the tr_TR change. I was for case-sensitive compares, but I think Dan had a bit of an issue with it. Dan, do you remember what it was?
15:40 <toofishes> i'm sure it is used by a handful of people 15:41 <toofishes> i know it isn't the prettiest, but i don't have a good reason to get rid of it (as it has been this way for a long time, we just fixed the issues with i <-> I) And vmiklos said it was probably used by many users : http://www.archlinux.org/pipermail/pacman-dev/2007-August/009168.html I doubt that many users use it, since every keywords present in the default pacman.conf have that same case format (RootDir, CacheDir, etc)
On Tue, Mar 11, 2008 at 11:23 AM, Xavier
On Tue, Mar 11, 2008 at 11:14:17AM -0500, Aaron Griffin wrote:
On Tue, Mar 11, 2008 at 10:10 AM, Xavier
wrote: So I would vote for only allowing case sensitive options in pacman.conf, to treat all locales equally, and cleaning the code :)
I originally brought this up to Dan when I made the tr_TR change. I was for case-sensitive compares, but I think Dan had a bit of an issue with it. Dan, do you remember what it was?
15:40 <toofishes> i'm sure it is used by a handful of people 15:41 <toofishes> i know it isn't the prettiest, but i don't have a good reason to get rid of it (as it has been this way for a long time, we just fixed the issues with i <-> I)
And vmiklos said it was probably used by many users : http://www.archlinux.org/pipermail/pacman-dev/2007-August/009168.html
I doubt that many users use it, since every keywords present in the default pacman.conf have that same case format (RootDir, CacheDir, etc)
Glad you brought this up and it got a bunch of responses. Looks like I am a bit in the minority here, and we probably would be OK just doing the CamelCase option names *if* we document this point in pacman.conf.5. Xavier, want to toss a patch into the mix here that cleans up the strcmp() usage AND documents the case-sensitivity in the manpage? -Dan
On Tue, Mar 11, 2008 at 12:59:36PM -0500, Dan McGee wrote:
Glad you brought this up and it got a bunch of responses.
Looks like I am a bit in the minority here, and we probably would be OK just doing the CamelCase option names *if* we document this point in pacman.conf.5.
Xavier, want to toss a patch into the mix here that cleans up the strcmp() usage AND documents the case-sensitivity in the manpage?
Yes, I also felt like it should be mentioned there, and I began to look at the man page, but wasn't sure in which section it belonged. Maybe at the end of the Description section ? Or the beginning of Options? I am not sure. Also, it will conflict with my CleanMethod patch, which one should I do first?
These case insensitive comparisons didn't work in some locales, like tr_TR
where upper(i) != I. So a second case sensitive comparison had to be made
for each directive.
Only keeping case sensitive comparisons make the code cleaner and treat all
locales equally.
Ref: http://www.archlinux.org/pipermail/pacman-dev/2008-March/011445.html
Signed-off-by: Chantry Xavier
On Tue, Mar 11, 2008 at 05:23:20PM +0100, Xavier
And vmiklos said it was probably used by many users : http://www.archlinux.org/pipermail/pacman-dev/2007-August/009168.html
I doubt that many users use it, since every keywords present in the default pacman.conf have that same case format (RootDir, CacheDir, etc)
i don't think anybody uses IGNOREPKG but if you type manually (and you know that it's case-insensiteve) then it's possible that you type ignorepkg. speaking of options which are not by default in the pacman config.
On Wed, Mar 12, 2008 at 11:16:58PM +0100, Miklos Vajna wrote:
i don't think anybody uses IGNOREPKG but if you type manually (and you know that it's case-insensiteve) then it's possible that you type ignorepkg. speaking of options which are not by default in the pacman config.
Well, Nagy already proposed to add all options in pacman.conf, but it might not be accepted for the arch pacman package. Even if it isn't, I don't think that's a big problem. pacman will fail saying ignorepkg is an unknown keyword, and the user should be able to figure out his mistake from there.
participants (7)
-
Aaron Griffin
-
Chantry Xavier
-
Dan McGee
-
Miklos Vajna
-
Nagy Gabor
-
Roman Kyrylych
-
Xavier