git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] difftool: honor --trust-exit-code for builtin tools

From
David Aguilar <davvid@gmail.com>
Date
Nov 16, 2014, 02:36 UTC
Message-ID
<20141116023609.GA74487@gmail.com>
In-Reply-To
<CAHYJk3Q9tcS+o0hDnDz24ysSKkL6m16OmhyHuj=W88VQjTximw@mail.gmail.com>
On Sun, Nov 16, 2014 at 02:51:11AM +0100, Mikael Magnusson wrote:
Show 34 quoted lines
> On Fri, Nov 14, 2014 at 10:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > David Aguilar <davvid@gmail.com> writes:
> >
> >> run_merge_tool() was not setting $status, which prevented the
> >> exit code for builtin tools from being forwarded to the caller.
> >>
> >> Capture the exit status and add a test to guarantee the behavior.
> >>
> >> Reported-by: Adria Farres <14farresa@gmail.com>
> >> Signed-off-by: David Aguilar <davvid@gmail.com>
> >> ---
> >>  git-mergetool--lib.sh | 1 +
> >>  t/t7800-difftool.sh   | 5 +++++
> >>  2 files changed, 6 insertions(+)
> >>
> >> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> >> index a40d3df..2b66351 100644
> >> --- a/git-mergetool--lib.sh
> >> +++ b/git-mergetool--lib.sh
> >> @@ -221,6 +221,7 @@ run_merge_tool () {
> >>       else
> >>               run_diff_cmd "$1"
> >>       fi
> >> +     status=$?
> >>       return $status
> >>  }
> >
> > Thanks for a quick turn-around.  As a hot-fix for what is already in
> > -rc I am fine with this fix but the patch makes me wonder if $status
> > as a global shell variable has any significance.
> 
> $status is an alias for $? in zsh, and so cannot be assigned to. But
> other than that I don't think it holds any meaning and should be fine
> in a .sh script.

Thanks for the heads-up ~ this is even more reason to cleanup the script a bit.

If we still need a local variable for it in a few places then I'll call it $rc instead, but it'll only be used for local things rather than its current global usage.

-- 
David
Previous: Mikael MagnussonNext: Junio C Hamano
Message 5 of 8 in “difftool: honor --trust-exit-code for builtin tools”
  1. difftool: honor --trust-exit-code for builtin toolsDavid Aguilar, Nov 14, 2014
  2. Junio C HamanoNov 14, 2014
  3. David AguilarNov 14, 2014
  4. Mikael MagnussonNov 16, 2014
  5. David AguilarNov 16, 2014
  6. Junio C HamanoNov 16, 2014
  7. Aaron SchrabNov 17, 2014
  8. Andreas SchwabNov 16, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.