I was trying to use "darcs record" with the -l option to look for new files, and
forgetting that -m requires an argument.
{{{
$ darcs record -ml
No changes!
$ darcs record -m -l
No changes!
$ darcs record -m

darcs failed:  option `-m' requires an argument PATCHNAME
}}}
In the last case, the error message is correct. I find this confusing because in
the first two cases, darcs prints out "No changes!" when there are changes. The
problem is, it seems, that -m is combined with another option but the argument
for -m is omitted, which results in "darcs record -ml" being operationally the
same as "darcs record -m". I think that all three command lines above should
print out the same error message.


My version info:

$ darcs --exact-version
darcs compiled on Jul  4 2006, at 20:08:54
# configured Fri Jun 16 14:55:21 EDT 2006
./configure --no-create --no-recursion

Context:

[TAG 1.0.8
Tommy Pettersson <ptp@lysator.liu.se>**20060616160213]

On Fri, Jan 19, 2007 at 07:24:20PM +0000, Kirsten Chevalier wrote:
> I was trying to use "darcs record" with the -l option to look for new files, and
> forgetting that -m requires an argument.
> {{{
> $ darcs record -ml
> No changes!
> $ darcs record -m -l
> No changes!
> $ darcs record -m
> 
> darcs failed:  option `-m' requires an argument PATCHNAME
> }}}
> In the last case, the error message is correct. I find this confusing because in
> the first two cases, darcs prints out "No changes!" when there are changes. The
> problem is, it seems, that -m is combined with another option but the argument
> for -m is omitted, which results in "darcs record -ml" being operationally the
> same as "darcs record -m". I think that all three command lines above should
> print out the same error message.

When I run

darcs record -ml

darcs creates a patch with name "l".

darcs record -m -l

creates a patch with name "-l".

So I guess this is a correct parsing according to getopt, and I'd not rate
this as a bug.  It's a little confusing, but this is the way many (most?)
programs handle their command-line parsing, so I don't think it's a bad
idea to use the "standard" command-line parsing approach.

David

On 1/19/07, David Roundy <bugs@darcs.net> wrote:
> When I run
>
> darcs record -ml
>
> darcs creates a patch with name "l".
>
> darcs record -m -l
>
> creates a patch with name "-l".
>
> So I guess this is a correct parsing according to getopt, and I'd not rate
> this as a bug.  It's a little confusing, but this is the way many (most?)
> programs handle their command-line parsing, so I don't think it's a bad
> idea to use the "standard" command-line parsing approach.

Perhaps, then, rather than rating it as a bug, it could be a wishlist
item to prompt "Did you really mean to create a patch with a
single-letter name?" when someone does this. I mean, I think it's
likely that people usually don't want to give their patches
single-letter names or names beginning with a "-". The common case,
IMO, is someone being forgetful like me, rather than someone who
really wants to name a patch "-l" -- so that case should be handled
gracefully while still allowing for the "standard Unix" behavior.

Cheers,
Kirsten

On Fri, Jan 19, 2007 at 12:47:36PM -0800, Kirsten Chevalier wrote:
> Perhaps, then, rather than rating it as a bug, it could be a wishlist
> item to prompt "Did you really mean to create a patch with a
> single-letter name?" when someone does this. I mean, I think it's
> likely that people usually don't want to give their patches
> single-letter names or names beginning with a "-". The common case,
> IMO, is someone being forgetful like me, rather than someone who
> really wants to name a patch "-l" -- so that case should be handled
> gracefully while still allowing for the "standard Unix" behavior.

Okay, when you put it that way it makes sense.

Except that an interactive prompt will only be appropriate if --all is not
specified, since users who use --all expect darcs to run
non-interactively.

So rather than modifying the command-line parsing, we'd put a special check
in darcs record.  Which would maybe exit with an error (stating that to
create patches with single-character names or names starting with '-' the
user should use --edit-long-comment) or prompt for confirmation.

David

On 1/19/07, David Roundy <bugs@darcs.net> wrote:
>
> Okay, when you put it that way it makes sense.
>
> Except that an interactive prompt will only be appropriate if --all is not
> specified, since users who use --all expect darcs to run
> non-interactively.
>
> So rather than modifying the command-line parsing, we'd put a special check
> in darcs record.  Which would maybe exit with an error (stating that to
> create patches with single-character names or names starting with '-' the
> user should use --edit-long-comment) or prompt for confirmation.

Yes, I think that would be the way to go. It seems like this would
prevent a lot of careless mistakes while preserving all functionality.

Cheers,
Kirsten

I attempted to address this as part of learning Haskell, but my attached patch
is not sufficient. It has two remaining problems:

- It doesn't yet cover the case of a name that starts with a dash and looks like
a flag
- It intervenes /after/ the user selects patches, but it should really intervene
/before/ patch selection, to avoid wasting the users time for effort that will
be thrown away. However, I couldn't figure out how to reference the patch name
in "record_cmd", so I have given up for now.

Attachments

• bug_395.patch

I just sent a patch which fixes the shell scripts in the test suite which used a
single character patch name.

-ml (-m followed by one flag) seems a bit of a special case. What about -mlv ? Is 
it worth the trouble ? A patch name beginning with - does seem worth catching.

Another approach might be to succeed and just print the warning:
  Warning: I don't think you meant -m -l! ('-l' is a suspiciously short patch name)

followed by (if a patch is recorded)
  You might consider 'darcs unrecord' to safely unrecord this patch, or
alternatively, 'darcs amend-record --edit-long-comment' to change its name.

The principle here being that offering an undo (trash, recycle bin) is generally
nicer than prompting the user ('do you really want to delete the file?'),
because the user will just reflexively hit 'y' and 2 seconds later say "crap,I
didn't mean that".

Also, I'm thinking that the annoyance to forgetful-self about having to unrecord
is less (especially considering that we give a friendly reminder about how to
recover), than that which is experienced from somebody that is frustrated from
using a really short patch name when he/she really meant it.

[Warning! I don't actually know anything about UI; I just read a book... so all
of this could just be hot air]

On Thu, Jan 31, 2008 at 07:37:33PM -0000, Eric Kow wrote:
> Another approach might be to succeed and just print the warning:
>   Warning: I don't think you meant -m -l! ('-l' is a suspiciously short patch name)
> 
> followed by (if a patch is recorded)
>   You might consider 'darcs unrecord' to safely unrecord this patch, or
> alternatively, 'darcs amend-record --edit-long-comment' to change its name.

That sounds good to me.
-- 
David Roundy
Department of Physics
Oregon State University

The following patch updated the status of issue395 to be resolved:

* resolve issue395: warn the user when the patch name looks like a command line option