zsh-workers
 help / color / mirror / code / Atom feed
From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: Jacob Gelbman <gelbman@gmail.com>
Cc: zsh-workers@zsh.org
Subject: Re: Completion script for the ctags program
Date: Wed, 3 Mar 2021 20:02:15 +0000	[thread overview]
Message-ID: <20210303200215.GA11821@tarpaulin.shahaf.local2> (raw)
In-Reply-To: <20210224142437.GC9342@tarpaulin.shahaf.local2>

Jacob, ping?  Is a followup patch in the offing?

As already mentioned, I think some of the review points below are
release blockers.

Daniel


Daniel Shahaf wrote on Wed, Feb 24, 2021 at 14:24:37 +0000:
> Jacob Gelbman wrote on Wed, Feb 24, 2021 at 01:20:24 -0600:
> > #compdef ctags
> 
> apt-file(1) on Debian stable shows a few more names:
> 
> arduino-ctags: /usr/bin/arduino-ctags
> emacs-bin-common: /usr/bin/ctags.emacs
> emacs-bin-common: /usr/bin/etags.emacs
> exuberant-ctags: /usr/bin/ctags-exuberant
> universal-ctags: /usr/bin/ctags-universal
> xemacs21-bin: /usr/bin/etags.xemacs21
> 
> I assume at least some of these should be added to the #compdef line.  Would
> you do the honours?
> 
> >     "--alias-<lang>=[add a pattern detecting a name, can be used as an alt name for lang]:pattern"
> 
> As Oliver said, literal angle brackets in the option name to be
> completed aren't especially helpful.  In fact, I'll go as far as to say
> I don't want users to run into it in released code.  Please change them.
> 
> You can use _call_program with --list-languages to generate the right set of
> option names dynamically.
> 
> > elif [ "$_ctags_type" = "exuberant" ]; then
> >   arguments=(
> >     "-a[append to tags file]"
> >     "-B[use backward searching patterns (?...?)]"
> >     "-e[output tag file for use with emacs]"
> >     "-f[write tags to specified file. - is stdout]:file:_files"
> 
> Is the argument to the -f option allowed to be pasted to it?  If so, s/-f/-f+/.
> 
> Also, s/:file:/:output file:/.  That part of the string is a user-facing
> message, so the extra detail is helpful.
> 
> Also, you can drop the "- is stdout" part.  The descriptions are only
> a summary of the functionality; they aren't meant to be a complete copy of
> the manual.
> 
> >     "-F[use forward searching patterns (/.../)]"
> >     "-h[specify list of file extensions to be treated as include files]:"
> 
> Write something after the colon.
> 
> >     "-I[a list of tokens to be specifically handled is read from either the command line or the specified file]:"
> 
> The thing in brackets doesn't describe the action of the option.  Please edit.
> 
> >     "-L[a list of input file names is read from the specified file. - is stdin]:file:_files"
> 
> Rephrase in the imperative.
> 
> >     "-R[equivalent to --recurse]"
> 
> This is normally rendered as:
> 
>       '(-r --recurse)'{-R,--recurse}'[description]'
> 
> >     "--fields=[include selected extension fields (flags afmikKlnsStz)]:flags"
> 
> Recommend to move the afmikKlnsStz thing to after the colon, so it'll be
> shown at a more appropriate point.  Also, it would be helpful to display
> descriptions to the flags using, e.g., «compset» (for the leading plus
> sign) followed by «_values -s ''».
> 
> >     "--file-scope=[should tags scoped only for a single file be included in output]:bool:(yes no)"
> >     "--filter=[behave as a filter, reading file names from stdin and writing tags to stdout]:bool:(yes no)"
> >     "--filter-terminator=[specify string to print to stdout following the tags for each file parsed when --filter is enabled]:string"
> >     "--format=[force output of specified tag file format]:level"
> >     "--help[help text]"
> 
> "help text" is just a noun phrase.  Please use complete decsriptions.
> 
> Please use exclusions if needed («'(--foo)--bar[baz]'»).
> 
> >     "--language-force=[force all files to be interpreted using specified language]:language:->language"
> >     "--languages=[restrict files scanned to these comma-separated languages]:language:->languages"
> 
> Can't say I'm a fan of having two states that differ by a single letter,
> but so be it.
> 
> >     "--recurse=[recurse]:bool:(yes no)"
> 
> Fix the bracketed description.
> 
> > _arguments $arguments
> 
> Pass any arguments to _arguments that may be needed (for
> instance, -s).
> 
> > if [[ "$state" = language* ]]; then
> >   local -a languages
> >   languages=(`ctags --list-languages | cut -d" " -f1`)
> 
> Use _call_program and $service.
> 
> >   if [ "$state" = "language" ]; then
> >     _wanted languages expl language compadd $languages
> 
> Don't pass unsanitized command output to a builtin.  In this case,
> «compadd -a languages» would do.
> 
> >   elif [ "$state" = "languages" ]; then
> >     _values -s , languages $languages
> 
> Don't pass unsanitized command output to a builtin.  I don't know the
> fix off the top of my head.
> 
> Thanks for the patch, and especially for adding exubertant and BSD ctags
> support!
> 
> Daniel
> 
> 
> > 
> > 
> > 
> > 
> > > On Feb 23, 2021, at 10:45 PM, Jacob Gelbman <gelbman@gmail.com> wrote:
> > > 
> > > Hey, thanks for looking at the script and adding it to the repo, although I think some of got pasted in wrong. There’s a lot to writing completion functions and I’m still not 100% sure how to do it right.
> > > 
> > >> On Feb 23, 2021, at 3:39 PM, Oliver Kiddle <opk@zsh.org> wrote:
> > >> 
> > >> Jacob Gelbman wrote:
> > >>> I wrote a completion script for the ctags program. Someone might be able to use it:
> > >> 
> > >> Which ctags!?
> > > 
> > > I have Universal Ctags 5.9.0
> > > 
> > >> This doesn't match what I have installed on any of my systems. There
> > >> are multiple implementations of ctags, with it often being just a link
> > >> to etags - for which there is a completion albeit not a well maintained
> > >> one. One of the main reasons, a completion doesn't already exist is
> > >> that it would ideally need to detect the variant and at least have sane
> > >> fallbacks for variants that aren't handled. It could be useful to check
> > >> what the existing _etags is handling - that might be the exhuberant or
> > >> emacs variant.
> > >> 
> > > 
> > > I located a few other ctags on my computers, I have BSD ctags that comes by default on the mac. Exuberant Ctags 5.8. and there’s etags that comes with emacs. I can probably add an if statement based on the output of ctags —version, and modify the function from that. If it’s etags, I’ll just:
> > > 
> > > _comps[ctags]=“_etags”; _etags
> > > 
> > > And exit.
> > > 
> > >> In general, please follow the conventions outlined in
> > >> Etc/completion-style-guide in the zsh source distribution. For example,
> > >> completion functions usually use just 2 spaces for indentation.
> > >> 
> > >>> #compdef ctags
> > >>> 
> > >>> local state
> > >> 
> > >> If you use states, you need to also handle the context which means
> > >> either passing -C to _arguments and setting up $curcontext or declaring
> > >> context local and passing it to later functions like _values.
> > > 
> > > The -C argument and the context/curcontext variables are confusing me, a lot.
> > > 
> > >> 
> > >>>   "--alias-<lang>=[add a pattern detecting a name, can be used as an alt name for lang]:pattern" \
> > >>>   "--input-encoding-<lang>=[specify encoding of the <lang> input files]:encoding" \
> > >>>   "--kinddef-<lang>=[define new kind for <lang>]:kind" \
> > >>>   "--kinds-<lang>=[enable/disable tag kinds for <lang>]:kind" \
> > >> 
> > >> These would not complete especially helpfully. I suspect that <lang> there is
> > >> supposed to be substituted.
> > > 
> > > They’d show up in the menu when you press tab, but if I filled in the actual values, the list would be too long.
> > > 
> > >> 
> > >>> if [ "$state" = "language" ]; then
> > >>>   compadd `ctags --list-languages | cut -d" " -f1`
> > >> 
> > >> It would be nicer to use a description by calling for example, _wanted
> > >> here.
> > > 
> > > I can do that.
> > > 
> > >> 
> > >>> elif [ "$state" = "languages" ]; then
> > >>>   _values -s , "languages" `ctags --list-languages | cut -d" " -f1`
> > >>> fi
> > >> 
> > >> I'd probably use _sequence here as it is smaller and simpler. But
> > >> _values is fine if none of the languages contain characters that need
> > >> quoting from it.
> > > 
> > > This too.
> > > 
> > >> 
> > >> The return status from this function will not be correct in all cases.
> > >> This can have effects like approximate completion being activated
> > >> despite matches having been added by earlier completers. Where states
> > >> are needed, you nearly always need to either save the status from
> > >> _arguments, typically via a ret variable or check $compstate[nmatches]
> > >> on exit.
> > >> 
> > >> Oliver
> > > 
> > 
> > 
> 


  parent reply	other threads:[~2021-03-03 20:02 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-23  3:11 Jacob Gelbman
2021-02-23 10:11 ` Peter Stephenson
2021-02-23 22:20   ` Oliver Kiddle
2021-02-23 22:45     ` Bart Schaefer
2021-02-23 23:51       ` Oliver Kiddle
2021-02-24  0:52         ` Bart Schaefer
2021-02-24 13:47           ` Daniel Shahaf
2021-02-23 23:05     ` Mikael Magnusson
2021-02-23 21:39 ` Oliver Kiddle
2021-02-24  4:45   ` Jacob Gelbman
2021-02-24  7:20     ` Jacob Gelbman
2021-02-24  9:26       ` Peter Stephenson
2021-02-24 14:24       ` Daniel Shahaf
2021-02-24 18:58         ` Jacob Gelbman
2021-02-24 19:01           ` Bart Schaefer
2021-03-03 20:02         ` Daniel Shahaf [this message]
2021-03-03 20:39           ` Jacob Gelbman
2021-03-03 21:40             ` Peter Stephenson
2021-03-03 22:06             ` Daniel Shahaf
2021-03-03 22:08           ` Jacob Gelbman
2021-03-03 23:28             ` Aaron Schrab
2021-03-03 23:43               ` Daniel Shahaf
2021-03-03 23:35             ` Daniel Shahaf
2021-03-07 19:18         ` Jacob Gelbman
2021-03-07 21:42           ` Daniel Shahaf
2021-03-07 21:57             ` Jacob Gelbman
2021-03-07 22:10               ` Daniel Shahaf
2021-03-11 16:15                 ` Daniel Shahaf
2021-03-11 17:08                   ` Jacob Gelbman
2021-03-20  1:43                     ` Lawrence Velázquez
2021-03-27 16:14                       ` Lawrence Velázquez
2021-03-27 20:43                         ` Daniel Shahaf
2021-03-28 23:29                     ` Oliver Kiddle
2021-03-29  8:54                       ` Peter Stephenson
2021-03-29 15:07                         ` EOL normalization? (Was: Completion script for the ctags program) Lawrence Velázquez
2021-03-29 15:34                           ` Daniel Shahaf
2021-03-29 15:41                             ` Lawrence Velázquez
2021-02-24 21:54       ` Completion script for the ctags program dana

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210303200215.GA11821@tarpaulin.shahaf.local2 \
    --to=d.s@daniel.shahaf.name \
    --cc=gelbman@gmail.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).