git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Jeff King <peff@peff.net>
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 3/3] completion: offer ctags symbol names for 'git log -S', '-G' and '-L:'
Date: Thu, 30 Mar 2017 12:06:56 +0200	[thread overview]
Message-ID: <20170330100656.18766-1-szeder.dev@gmail.com> (raw)
In-Reply-To: <20170324005256.ji2wijhyqnwbpp5t@sigill.intra.peff.net>


On Fri, Mar 24, 2017 at 1:52 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Mar 23, 2017 at 04:38:39PM +0100, SZEDER Gábor wrote:
> 
> > Just like in the case of search patterns for 'git grep', see 29eec71f2
> > (completion: match ctags symbol names in grep patterns, 2011-10-21)),
> > a common thing to look for using 'git log -S', '-G' and '-L:' is the
> > name of a symbol.
> > 
> > Teach the completion for 'git log' to offer ctags symbol names after
> > these options, both in stuck and in unstuck forms.
> 
> I think this makes sense and is an improvement over the status quo.
> 
> There are two gotchas with completing "-L" like this:

Reordering your gotchas for convenience...

>   2. The function name is actually a regex, so you can get bit when your
>      function name is a subset of another.

... and when the longer name comes first in the given file.  Yeah, I
was bitten by that a few of times, too :)

However, I found that completing symbol names can be quite helpful in
this case, because now I see that there are longer symbol names
starting with what I'm looking for while typing/completing the
command, and then I can just delete the trailing ':' and add a '\(:'
or '\ :' depending on language and coding style right away.  Of course
it doesn't help, if the function name in question is the suffix or a
substring in the middle of another one, but that seems to be even
rarer, or at least I've yet to come across such a case in practice.

>   1. You still have to come up with the filename yourself for "-L".

I was already quite satisfied that both the symbol name and the 
filename can be TAB completed...  but right, in most cases the
function name uniquely determines the filename, and even when it
doesn't, it still significantly limits the possibilities.  Hmhm.

Listing the funcname:filename together is not a good idea in my
opinion.  A small change to __git_match_ctag()'s awk script can show
why:

-               /^${1//\//\\/}/ { print pfx \$1 sfx }
+               /^${1//\//\\/}/ { print pfx \$1 \":\" \$2 sfx }

The amount of text flooding my terminal on completion is just
overwhelming (and I haven't even tried it in a Java project :), and it
breaks the "helpful" use-case with longer matching symbol names I
described above (it's much harder to see that there are longer
matching symbol names and I would have to backtrack the whole filename
to modify the funcname).  It also breaks symbol name completion for
'-G' and '-S', of course, so further changes would be necessary to
deal with that.

OTOH, the proof-of-concept patch at the bottom shows how we could
start doing filename completion based on the ctags file, and I think
it's really convenient to use.  Alas, it doesn't work when the
funcname is not on its own, e.g. ends with that disambiguating '\(:'
from above, and then Bash falls back to its own filename completion.
However, if I may extrapolate from my ~/.bash_history, this would
still help the great majority of the cases.

> I have a script (below) which makes this easier (and I complete its
> argument using the tags file).  It's probably too gross to even go into
> contrib, but I thought I'd share.

Perhaps 'git log -L' could be enhanced to just DWIM when it gets only
'-L:func:', and show the log of that function, wherever it is defined.
So instead of complaining about the missing filename, it could run
'grep <func>' with a bit of magic to find the filename where the
matching function is declared, and search in the history of that file.

But then again, 'git log -L' could be enhanced in so many ways...


Gábor


 -- >8 --

Subject: [PATCH] [PoC] completion: list only relevant filenames after 'git log
 -L:funcname:'

---
 contrib/completion/git-completion.bash | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d6f25a7e0..8b58096e4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1678,6 +1678,13 @@ __git_log_shortlog_options="
 __git_log_pretty_formats="oneline short medium full fuller email raw format:"
 __git_log_date_formats="relative iso8601 rfc2822 short local default raw"
 
+__git_filename_for_symbol ()
+{
+	local symbol="$1" tags="$2"
+
+	awk "/^${symbol//\//\\}/ { print \$2 }" "$tags"
+}
+
 _git_log ()
 {
 	__git_has_doubledash && return
@@ -1689,7 +1696,11 @@ _git_log ()
 	fi
 	case "$prev,$cur" in
 	-L,:*:*)
-		return	# fall back to Bash filename completion
+		local symbol="${cur#:}"
+		symbol="${symbol%%:*}"
+		__gitcomp_nl "$(__git_filename_for_symbol "$symbol" tags)" \
+			"" "${cur#:$symbol:}"
+		return
 		;;
 	-L,:*)
 		__git_complete_symbol --cur="${cur#:}" --sfx=":"
@@ -1746,7 +1757,11 @@ _git_log ()
 		return
 		;;
 	-L:*:*)
-		return	# fall back to Bash filename completion
+		local symbol="${cur#-L:}"
+		symbol="${symbol%%:*}"
+		__gitcomp_nl "$(__git_filename_for_symbol "$symbol" tags)" \
+			"" "${cur#-L:$symbol:}"
+		return
 		;;
 	-L:*)
 		__git_complete_symbol --cur="${cur#-L:}" --sfx=":"
-- 
2.12.2.453.g223d29900


  reply	other threads:[~2017-03-30 10:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-23 15:38 [PATCH 0/3] completion: updates to ctags symbol names SZEDER Gábor
2017-03-23 15:38 ` [PATCH 1/3] completion: put matching ctags symbol names directly into COMPREPLY SZEDER Gábor
2017-03-24  0:37   ` Jeff King
2017-03-23 15:38 ` [PATCH 2/3] completion: extract completing ctags symbol names into helper function SZEDER Gábor
2017-03-24  0:40   ` Jeff King
2017-03-23 15:38 ` [PATCH 3/3] completion: offer ctags symbol names for 'git log -S', '-G' and '-L:' SZEDER Gábor
2017-03-24  0:52   ` Jeff King
2017-03-30 10:06     ` SZEDER Gábor [this message]
2017-04-01  8:50       ` Jeff King

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=20170330100656.18766-1-szeder.dev@gmail.com \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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.
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).