Re: Re* [PATCH] contrib/git-jump: cat output when not a terminal

From: Jeff King <peff@peff.net>
Date: May 12, 2020, 19:23 UTC
Message-ID: <20200512192323.GC54565@coredump.intra.peff.net>
In-Reply-To: <xmqqy2pyqv11.fsf_-_@gitster.c.googlers.com>

On Mon, May 11, 2020 at 09:46:34AM -0700, Junio C Hamano wrote:

> Lest we all forget...
> 
> -- >8 --
> Subject: git-jump: just show the list with the "--no-editor" option

Thanks for tying this up. It seems to work as advertised. A few nits:

> +edit=yes
> +
> +while	case "$#,$1" in

Tab between "while" and "case"?

Show 8 quoted lines

> +	0,*) break ;;
> +	*,--no-editor) edit=no ;;
> +	*,--*) usage >&2; exit 1 ;;
> +	*) break ;;
> +	esac
> +do
> +	shift
> +done

I found the use of "case" in the loop conditional a little unusual. I'd have probably written:

  while test $# -gt 0
  do
	case "$1" in
	--no-editor) edit=no ;;
	--*) usage >&2; exit 1 ;;
	*) break ;;
	esac
	shift
  done

Show 11 quoted lines

> @@ -75,4 +87,9 @@ tmp=`mktemp -t git-jump.XXXXXX` || exit 1
>  type "mode_$mode" >/dev/null 2>&1 || { usage >&2; exit 1; }
>  "mode_$mode" "$@" >"$tmp"
>  test -s "$tmp" || exit 0
> -open_editor "$tmp"
> +
> +case "$edit" in
> +yes)	open_editor "$tmp" ;;
> +no)	cat "$tmp" ;;
> +esac
> +

"diff --check" complains about the empty line.

It probably doesn't matter much, but we could skip the tempfile entirely in no-editor mode. I.e.:

  if test "$edit" = "no"
  then
    "mode_$mode" "$@"
  fi

  # otherwise set up trap, mktemp, etc

-Peff