darcs

Patch 2123 completion: use '--list-options' before any other option

Title completion: use '--list-options' before any other option
Superseder Nosy List gpiero
Related Issues
Status accepted Assigned To
Milestone

Created on 2020-12-17.17:13:08 by gpiero, last changed 2021-03-22.06:23:32 by ganesh.

Files
File name Status Uploaded Type Edit Remove
bash_completion_-do-not-use-provided-options-when-invoking-_darcs-___-__list_options_.dpatch dead gpiero, 2020-12-17.17:13:03 application/x-darcs-patch
fix-errors-in-zsh-completions_-add-comments.dpatch bf, 2020-12-25.09:59:29 application/x-darcs-patch
patch-preview.txt gpiero, 2020-12-17.17:13:03 text/x-darcs-patch
patch-preview.txt bf, 2020-12-23.16:06:22 text/x-darcs-patch
patch-preview.txt bf, 2020-12-25.09:59:29 text/x-darcs-patch
patch2123.dpatch gpiero, 2020-12-23.12:03:57 text/plain
unnamed bf, 2020-12-23.16:06:22 text/plain
unnamed bf, 2020-12-25.09:59:29 text/plain
zsh-completion_-use-___list_options_-before-any-other-option.dpatch bf, 2020-12-23.16:06:22 application/x-darcs-patch
See mailing list archives for discussion on individual patches.
Messages
msg22542 (view) Author: gpiero Date: 2020-12-17.17:13:03
I've verified the same issue exists with zsh (obviously, as the bash completion
has been copied from the zsh one), but I'd prefer to leave that to someone who
actually uses zsh.

1 patch for repository http://darcs.net:

patch a42f230a854f758371ca097cddbb822083fb7b52
Author: Gian Piero Carrubba <gpiero@rm-rf.it>
Date:   Thu Dec 17 18:22:13 CET 2020
  * bash_completion: do not use provided options when invoking `darcs ... --list-options`
  
  This is so to avoid that, e.g.:
  
  $ darcs record -m <Tab><Tab>
  
  results in a patch named '--list-options' being recorded.
Attachments
msg22543 (view) Author: bf Date: 2020-12-18.08:27:13
Hm, yes, I can confirm that the zsh completions have the same issue. 
This is clearly a bug and a severe one at that.

But your proposed solution seems a bit drastic in that it also 
disables a lot of useful functionality. What darcs outputs with --
list-options does depend on the existing command line flags. For 
instance 'darcs record -l --list-options' lists un-added files, 
while a 'darcs record --list-options' does not.

The real problem here is that the "--list-options" on the command 
line gets "swallowed" as an argument to "-m", so darcs does not 
recognize that it should merely list completions. I would like to 
have my cake and eat it, too.

I think the correct fix is to place the --list-options not after but 
before any option already present on the command line.
msg22547 (view) Author: bf Date: 2020-12-18.09:15:07
Note that options are not allowed to come before commands and 
subcommands. So we have to find the first argument that starts with "-
" (the end of the list if there are none) and insert the "--list-
options" at that point.
msg22548 (view) Author: gpiero Date: 2020-12-18.09:35:33
* [Fri, Dec 18, 2020 at 08:27:15AM +0000] Ben Franksen:
>What darcs outputs with -- list-options does depend on the existing 
>command line flags. For instance 'darcs record -l --list-options' lists 
>un-added files, while a 'darcs record --list-options' does not.

Didn't realize this (nor that --list-options also consumes the 
command line after it). Will amend.

>I think the correct fix is to place the --list-options not after but
>before any option already present on the command line.

That seems right.
msg22600 (view) Author: gpiero Date: 2020-12-23.12:03:59
Amended patch.

Tangentially, in src/Darcs/UI/Commands/Record.hs:255:

checkNameIsNotOption (Just name) True   =
     when (length name == 1 || (length name == 2 && head name == '-')) $ do
         confirmed <- promptYorn $ "You specified " ++ show name ++ " as the patch name. Is that really what you want?"
         unless confirmed $ putStrLn "Okay, aborting the record." >> exitFailure

Would it be sensible to check for a patch name starting with '-' without 
the length constraint?
Or maybe better:
length name == 1 ||
(length == 2 && head name == '-') ||
(length > 2 && take 2 name == "--" && isAlpha/isAlphaNum (name!!2))

I don't like to be burdened with useless prompts, but maybe sometimes 
they make sense...
Attachments
msg22601 (view) Author: bf Date: 2020-12-23.14:47:54
> Tangentially, [...]

Your proposed check for the patch name looks good, feel free to send a
patch.
msg22602 (view) Author: bf Date: 2020-12-23.16:06:22
Here is my corresponding change for zsh. It always amazes me how much one
can do with one or two lines of zsh parameter expansion.

1 patch for repository http://darcs.net/screened:

patch c36d5a2bdf3aa4cca96f5d778252ca50c87e4868
Author: Ben Franksen <ben.franksen@online.de>
Date:   Wed Dec 23 17:15:22 CET 2020
  * zsh completion: use '--list-options' before any other option
  
  See the corresponding change in the bash completion for details.
Attachments
msg22606 (view) Author: bf Date: 2020-12-25.09:59:29
1 patch for repository http://darcs.net/screened:

patch 43c823f6c858101ffbb237f44f3a3ad92ad79349
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Dec 25 11:07:36 CET 2020
  * fix errors in zsh completions, add comments
  
  This fixes a stupid cut-and-paste error where I used x instead of words.
  Also, replacing the first option arg works only if the is an option arg in
  the first place, we need to properly insert --list-options.
Attachments
msg22643 (view) Author: ganesh Date: 2021-02-22.22:30:41
These look reasonable though I haven't tried them out myself.
History
Date User Action Args
2020-12-17 17:13:08gpierocreate
2020-12-18 08:27:14bfsetmessages: + msg22543
2020-12-18 09:15:07bfsetmessages: + msg22547
2020-12-18 09:35:33gpierosetmessages: + msg22548
2020-12-23 12:03:59gpierosetfiles: + patch2123.dpatch
messages: + msg22600
2020-12-23 14:47:54bfsetmessages: + msg22601
2020-12-23 16:06:23bfsetfiles: + patch-preview.txt, zsh-completion_-use-___list_options_-before-any-other-option.dpatch, unnamed
messages: + msg22602
2020-12-23 19:31:18bfsetstatus: needs-screening -> needs-review
2020-12-25 09:58:39bfsettitle: bash_completion: do not use provided options when invo... -> completion: use '--list-options' before any other option
2020-12-25 09:59:30bfsetfiles: + patch-preview.txt, fix-errors-in-zsh-completions_-add-comments.dpatch, unnamed
messages: + msg22606
2021-02-22 22:30:41ganeshsetstatus: needs-review -> accepted-pending-tests
messages: + msg22643
2021-03-22 06:23:32ganeshsetstatus: accepted-pending-tests -> accepted