git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Max Rothman <max.r.rothman@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] completion: add missing completions for log, diff, show
Date: Wed, 08 Nov 2017 11:33:13 +0900
Message-ID: <xmqq7ev1mrdi.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <0102015f985d387e-f50183c4-4b49-4a9f-b365-2a86ba24bbed-000000@eu-west-1.amazonses.com>

Max Rothman <max.r.rothman@gmail.com> writes:

> From: Max Rothman <max-rothman@pluralsight.com>

Thanks.

>
> Teach git-log tab completion about the --no-* options for ease of use
> at the command line.
>
> Similarly, teach git-show tab completion about the --no-abbrev-commit,
> --expand-tabs, and --no-expand-tabs options.
>
> Also, teach git-diff (and all commands that use its options) tab
> completion about the --textconv and --indent-heuristic families of
> options. --indent-heuristic is no longer experimental, so there's no
> reason it should be left out of tab completion any more, and textconv
> seems to have simply been missed.

A couple of things that I found questionable in the above
descriptions are:

 * We do not write git subcommand names like git-foo these days, as
   nobody type them like so.

 * The patch is not teaching git-foo about completing its options.
   It teaches the bash completion about options for git subcommands
   it did not know about.

So perhaps

	The bash completion script knows some options to the "git
	log" only in the positive form (e.g. "--abbrev-commit") but
	not in their negated form (e.g. "--no-abbrev-commit").  Add
	them.

and similar?

> ---

Missed sign-off?

> @@ -1759,16 +1765,19 @@ _git_log ()
> ...
> -			--decorate --decorate=
> +			--decorate --decorate= --no-decorate
> ...
> @@ -2816,8 +2825,9 @@ _git_show ()
>  		return
>  		;;
>  	--*)
> -		__gitcomp "--pretty= --format= --abbrev-commit --oneline
> -			--show-signature
> +		__gitcomp "--pretty= --format= --abbrev-commit --no-abbrev-commit
> +			--oneline --show-signature --patch
> +			--expand-tabs --expand-tabs= --no-expand-tabs
>  			$__git_diff_common_options
>  			"
>  		return

It's a bit sad that the completion support does not know that "git
show" belongs to the "git log" family of commands.  A consequence of
this is that "git show --no-decorate" is perfectly acceptable but
needs to be taught separately to _git_show if we wanted to.

Perhaps some selected options _git_log understands may need to be
split $__git_log_ui_common_options [*1*], like we do for "git diff"
family with $__git_diff_common_options, and shared between _git_log
and _git_show.

I am mentioning this primarily as #leftoverbits and I do not want to
see such a change mixed into this patch, as it is totally outside
the scope of it.  It is (if you are inclined to do so) OK to make
this into two patch series, with 1/2 doing such a refactoring
without changing any externally visible functionality (i.e. just
move the ones _git_show knows about to $__git_log_ui_common_options,
and have _git_show and _git_log use it), and 2/2 adding more options
to that list to achieve what you wanted to do with this patch, though.

Thanks.


[Footnote]

*1* $__git_log_common_options is taken and it is about options
    common to "log/shortlog/gitk" but contains some that are not
    relevant to "git show", and that is why I am suggesting a
    separate and new variable.



  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-11 14:47 No log --no-decorate completion? Max Rothman
2017-10-11 18:09 ` Stefan Beller
2017-10-12 17:41   ` Max Rothman
2017-10-24 15:32     ` Max Rothman
     [not found]     ` <CAFA_24K99LkeJBKV7+a-m-m9PUZik49cOd40+cEn-6zCvGmMsQ@mail.gmail.com>
2017-10-24 15:32       ` Stefan Beller
2017-11-02 20:25         ` Max Rothman
2017-11-02 21:19           ` Stefan Beller
2017-11-07 20:31             ` Max Rothman
2017-11-07 20:48               ` Stefan Beller
2017-11-07 21:22                 ` [PATCH] completion: add missing completions for log, diff, show Max Rothman
2017-11-08  2:33                   ` Junio C Hamano [this message]
2017-11-08  3:30                     ` Max Rothman
2017-11-08  4:06                       ` Junio C Hamano

Reply instructions:

You may reply publically 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqq7ev1mrdi.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=max.r.rothman@gmail.com \
    /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

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox