Re: CVS update: subversion/subversion/client main.c

[Greg Hudson <gh...@MIT.EDU>]

Karl, a suggestion about the command-processing code in main.c: pull
out the non-filename arguments outside the while loop.  It will make
it clearer to the reader what's going on than your current
state-machine design does, and it won't require any more cases.

A further, independent suggestion: instead of forcing main() to know
about the syntax of particular commands (which will only get uglier as
time goes on), make the command desc include a number of non-filename
arguments--2 for propset, 1 for propget, 0 for most other commands.
Bundle the non-option arguments into an array of strings and then
iterate over the filename arguments like you do now.  When we need it,
a -1 in that field can mean "just give me all of the arguments in a
string array and I'll decide what to do with them."  Get rid of
svn_cl__command_id so that you won't get back into the trap of main()
having to know about specific commands.

(I can implement these, but you're actively working on the code and I
wouldn't want to step on that.)

Re: CVS update: subversion/subversion/client main.c

[Karl Fogel <kf...@galois.collab.net>]

Greg Hudson <gh...@MIT.EDU> writes:
> Karl, a suggestion about the command-processing code in main.c: pull
> out the non-filename arguments outside the while loop.  It will make
> it clearer to the reader what's going on than your current
> state-machine design does, and it won't require any more cases.

Agreed; a good idea, & I will do it tomorrow if it's not already done
when I get in. :-)

> A further, independent suggestion: instead of forcing main() to know
> about the syntax of particular commands (which will only get uglier as
> time goes on), make the command desc include a number of non-filename
> arguments--2 for propset, 1 for propget, 0 for most other commands.
> Bundle the non-option arguments into an array of strings and then
> iterate over the filename arguments like you do now.  When we need it,
> a -1 in that field can mean "just give me all of the arguments in a
> string array and I'll decide what to do with them."  Get rid of
> svn_cl__command_id so that you won't get back into the trap of main()
> having to know about specific commands.
> 
> (I can implement these, but you're actively working on the code and I
> wouldn't want to step on that.)

I agree -- this needs to be generalized.

Again, feel free to have at it if you wish; I won't be touching the
code until tomorrow morning.

-K