git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Danny Lin <danny0838@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: git develop <git@vger.kernel.org>
Subject: Re: [PATCH] git-prompt: fix sequencer/todo detection
Date: Mon, 28 Mar 2022 14:53:29 -0700	[thread overview]
Message-ID: <xmqqwngdzque.fsf@gitster.g> (raw)
In-Reply-To: <20220325145301.3370-1-danny0838@gmail.com> (Danny Lin's message of "Fri, 25 Mar 2022 22:53:01 +0800")

Danny Lin <danny0838@gmail.com> writes:

> Previous case does not correctly check the "p ..." pattern.
>
> Signed-off-by: Danny Lin <danny0838@gmail.com>
> ---
>  contrib/completion/git-prompt.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index db7c0068fb..8ae341a306 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -315,7 +315,7 @@ __git_sequencer_status ()
>  	elif __git_eread "$g/sequencer/todo" todo
>  	then
>  		case "$todo" in
> -		p[\ \	]|pick[\ \	]*)
> +		p[\ \	]*|pick[\ \	]*)
>  			r="|CHERRY-PICKING"
>  			return 0
>  		;;

It is obvious that the original code is *not* prepared to see 'p'
followed by whitespace followed by other things, but I am not sure
how the code in sequencer.c::todo_list_write_to_file() can choose
to pass flags & TODO_LIST_ABBREVIATE_CMDS to todo_list_to_strbuf().

Danny, do you have a reproduction recipe, preferrably one you can
turn into a new test in t9903-bash-prompt.sh?  Or was this found
merely by inspecting the code?

Dscho, as far as I can tell, builtin/rebase.c can set the bit in the
flags word when rebase.abbreviatecommands configuration is set, but
that configuration variable is about rebase and it shouldn't affect
how multi-step cherry-pick would work, should it?  I am wondering if
an uninitialized "flags" word, whose TODO_LIST_ABBREVIATE_CMDS bit
randomly was turned on, caused todo_list_to_strbuf() to write an
abbreviated insn in the todo file.  If so, the insn word being
abbreviated or fully spelled out would not affect the correctness,
but the flags word affects other things that are more crucial to
correctness, so...

Thanks.



  parent reply	other threads:[~2022-03-28 22:16 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-25 14:53 [PATCH] git-prompt: fix sequencer/todo detection Danny Lin
2022-03-26  0:15 ` Junio C Hamano
2022-03-28 21:53 ` Junio C Hamano [this message]
2022-04-07 21:45   ` Johannes Schindelin
2022-04-07 22:43     ` Junio C Hamano

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

  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=xmqqwngdzque.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=danny0838@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    /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://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).