[pacman-dev] [PATCH] scripts/library: rewrite parse_options

[Allan McRae]

On 18/08/11 10:26, Dave Reisner wrote:
> On Thu, Aug 18, 2011 at 10:22:18AM +1000, Allan McRae wrote:
>> On 18/08/11 09:34, Dave Reisner wrote:
>>> On Thu, Aug 18, 2011 at 08:57:55AM +1000, Allan McRae wrote:
>>>> On 18/08/11 08:27, Dan McGee wrote:
>>>>> On Thu, Jul 21, 2011 at 3:40 PM, Dave Reisner<d at falconindy.com>    wrote:
>>>>>> In addition to being what I feel is a cleaner and faster implementation,
>>>>>> we avoid the use of eval by normalizing option arguments into a global
>>>>>> array which is then set after a successful call to parse_options.
>>>>>>
>>>>>> This trims out the idea of having multiple arguments to a single option,
>>>>>> making our parsing algorithm a little more sane. We never took advantage of
>>>>>> this in makepkg (for the one option that feasibly supports it), and I
>>>>>> think we've overlooked a much simpler solution in pacman-key. Since
>>>>>> actions are limited to 1 at a time the leftover positional parameters
>>>>>> become the keys or keyfiles which are acted upon.
>>>>>>
>>>>>> Also added is a new test directory test/scripts with a harness for
>>>>>> parse_options, run as part of make check.
>>>>>>
>>>>>> Signed-off-by: Dave Reisner<dreisner at archlinux.org>
>>>>>> ---
>>>>>> Thoughts? I know Allan wasn't quite sold on this, as the downside is that we
>>>>>> sort of pigeonhole ourselves into using the non-optional parameter as arguments
>>>>>> to our action. This has zero effect on makepkg.
>>>>>>
>>>>>> I've also thought to add --option=arg syntax parsing, but I'm not sure we need
>>>>>> this.
>>>>>
>>>>> Allan, was deferring to you on this.
>>>>
>>>>
>>>> Well, I was deferring to you as Dave is right that I was never sold
>>>> on this...
>>>>
>>>> Unless I am missing something, this does have a minor effect on
>>>> makepkg.  In git "makepkg --pkg foo bar" builds only foo and bar
>>> >from a split package.
>>>
>>> And, imo, this introduces bizzare unexpected behavior. With the
>>> "standard" getopt{,_long}, passing something such as:
>>>
>>>    --pkg foo bar
>>>
>>> I'd expect bar to be completely unrelated to the flag. Not the case
>>> here, and this behavior isn't really documented clearly at all. We don't
>>> even properly handle arguments with whitespace anymore. Not such a big
>>> deal for makepkg, but I think it's reasonable that pacman-key might some
>>> day need to import a key from a file with a space in the name.
>>
>> I thought the whitespace issue was fixed but testing now I see it is
>> not.  That is definitely something that needs addressed.
>>
>>>> I guess with this patch we would have to quote the package names in
>>>> some way (like is needed on the current maint release).  So we would
>>>> need to revert changes made to the makepkg documentation when the
>>>> multiple arguments stuff was added.
>>>
>>> We can (should?) follow pacman here and separate multiple arguments by
>>> commas. I'm going to channel Dan and call out "consistency" here.
>>
>> You mean like "pacman -S pkg1,pkg2"?   Fairly sure we do not do
>> that! So I call consistency...  Where do we use commas in pacman
>> options?
>
> No I mean like pacman -Syu --ignore pkg1,pkg2
>
> But, that's a good point about the "flexibility" of the non-option
> parameters. Just like I'm proposing for pacman-key, we generalize the
> term for pacman and call them "targets". They could be a repo, a
> file, a sync/local package... They don't belong to any particular
> option, but they're acted upon by the singular specified action.
>

I'll just add a final note that we are losing the ability to ever do 
anything like "pacman-key --add key1 key2 --delete key3" (at least with 
that syntax).  You might say that is a good thing, but there is a long 
standing bug report for universal transactions in pacman (the original 
4.0 target!) allowing us to do the analogous "pacman -S pkg1 pkg2 -R pkg3".

I'm happy taking away the ability to do one under the argument of 
consistency as long as the other is taken off the table too.

Allan