[Toybox] Mutually Exclusive Arguments. Bug?

[Rob Landley]

On 06/05/2012 11:57 AM, briggers at ninthfloor.org wrote:
>> Which is exactly why I need to define groups in a different way.
>>
>> The inability to refer to things which haven't been defined yet is
>> a design limit in parse_optflaglist(), and the easy way to fix it
>> is to define a new syntax that goes at the _end_ of the string to
>> refer to relationships between groups of options. Which is what
>> I'm doing.
> 
> I hope you don't mind my saying this, but do you think defining a new
> syntax is a good idea?

*shrug* I defined the old one. It's loosely derived from man 3 getopt,
but that doesn't handle longopts, fill out an array, handle multiple
data types, and so on.  Plus this nonsense from man 3 getopt:

  If getopt() finds an option character in argv that was not included
  in optstring,  or  if it detects a missing option argument, it
  returns '?' and sets the external variable optopt to the actual
  option  character.If  the  first  character  (following any optional
  '+' or '-' described above) of optstring is a colon (':'), then
  getopt() returns ':' instead of  '?'  to  indicate  a  missing
  option  argument.

I don't do anything remotely like that.

> It seems the problem is with the implementation
> rather that the current syntax itself.

Partly, but also the old syntax makes it hard to work out the bit values
for options.

In theory, you take the letters in the string, ignoring punctuation and
reading right to left, and that's your bits in powers of 2.  So the
rightmost one is "1" (1<<0), the next one to the left is "2" (1<<1), the
next one to the left is 4 (1<<2), and so on.

Now here's what the cp string _used_ to look like:

  USE_CP(NEWTOY(cp, "<2vslrR+rdpa+d+p+rHLPif", TOYFLAG_BIN))

The rightmost ones are easy: f=1, i=2, P=4, L=8, H=16... but then "r=32"
is wrong. It's actually a=32, and you have to read left through 7
characters to figure that out.

New format:

  "<2vslrRdpaHLPif[+rR][+adpr]"

To me it's a little more obvious what the actual option letters are out
of that. The letters in square brackets define relationships between
existion options, they don't define new options.

> The current syntax basically
> already supports mutually exclusive arguments, the implementation
> doesnt. Defining a new syntax not only involes implementing the new
> syntax but might also entail going back and fixing a lot of the old code
> (And all the documentation). With the old syntax you could just redo the
> implementation to build the structures for all the options first before
> populating the additional options, without breaking any of the toys code.

The patch I sent changed the documentation in the code, I need to update
code.html anyway (for the dirtree stuff).

The existing toys aren't using it much:

  sed -n 's/.*TOY([^"]*\("[^"]*"\).*/\1/p' toys/*.c \
    | grep '[a-zA-Z][+-^]'

Two hits:
  "tl^L^"
  "tl^L^"

Both in netcat.c.

> Unless of course you have grander plans that would benefit from a rewrite.

Basically, yeah.

>> For right now, implement it with an if() in main to test for
>> it, and I'll clean it up to have the option parsing do it
>> automatically once the infrastructure's in.  Sound good?
> 
> My plan was to go about adding missing functionality to the commands
> already present. If you intend to make infrastructure changes I might
> just hold off till those are finished.

I always reserve the right to go back and improve the infrastructure.
Don't let that stop other development work, I'll do the necessary
cleanups to existing code as I go.

Since the +x change isn't checked in yet, if you want to submit a patch
still using it, it's easy enough for me to convert it over when I get
the new infrastructure done.

> If you already have a plan/design
> for the infrastructure changes and only need to implement them, let me
> know and I can help out with that instead.

Well, my _current_ plan is to clean up for a release, which is why I
haven't checked in the redo here. I'm not letting new development block
the release (which I hoped to get done last week but didn't, and now
hope to get done this weekend.)

I checked in the cp change because there I could just change the C code
to not use the + syntax at all, without making it noticeably bigger. In
fact I suspect the + syntax could always be redone as if (flags &
(FLAG_a|FLAG_b|FLAG_c)) without making the code bigger since the | on
constants gets resolved at compile time according to c99. But some other
things (tar cx) need the option parsing to catch conflicts, or have a
lot of plumbing in the function's main() to sanity check the arguments.

In general, I'm constantly re-evaluating "is the way I planned to do it
the best way to continue given what I know now"? The infrastructure's
stabilized a lot along the way, but I just rewrote dirtree because
things like ls couldn't use what I had, and now they can.

I'm not good enough at this to have a "grand plan" that survives contact
with the enemy. I figure out stuff as I go along and clean it up once
I've figured out what I need to do. The weird inside-out ordering of ls
directory traversal made me throw out the existing dirtree code (which
was written to make cp work, with an eye towards future use in tar and
mke2fs and zip and such). But if I'd done it for ls first, I'd have then
rewritten it when cp needed stuff. (And chgrp needed depth-first
traversal...)

I'll post my todo list for the release in a separate message.

Rob
-- 
GNU/Linux isn't: Linux=GPLv2, GNU=GPLv3+, they can't share code.
Either it's "mere aggregation", or a license violation.  Pick one.