Re: [PATCH 4/8] perf/run: use $default_value instead of $4

Junio C Hamano [off-list ref] writes:

If you want to be able to use this helper to specify a default value
of an empty string (which the orignal that used $4 did), then the
previous hunk must be corrected so that it does not unconditionally
set default_value to $4.  Perhaps like

	if test -n "${4+x}"
	then
		default_value=$4
	else
		unset default_value || :
	fi

or something.

And if you do not care about passing an empty string as the default
value, then the earlier hunk can stay the same

	default_value=$4

but the eval part should become something like

	test -n "$default_value" && eval ...

Given that you are planning to add yet another optional argument to
the helper, and when you pass that variable, you'd probably need to
pass "" as the "default" that is not exported, perhaps this "give up
ability to pass an empty string as the default" approach may be the
only thing you could do.

On Wed, Dec 13, 2017 at 9:54 PM, Junio C Hamano [off-list ref] wrote:

Junio C Hamano [off-list ref] writes:

quoted

If you want to be able to use this helper to specify a default value
of an empty string (which the orignal that used $4 did), then the
previous hunk must be corrected so that it does not unconditionally
set default_value to $4.  Perhaps like

      if test -n "${4+x}"
      then
              default_value=$4
      else
              unset default_value || :
      fi

or something.

And if you do not care about passing an empty string as the default
value, then the earlier hunk can stay the same

        default_value=$4

but the eval part should become something like

        test -n "$default_value" && eval ...

Given that you are planning to add yet another optional argument to
the helper, and when you pass that variable, you'd probably need to
pass "" as the "default" that is not exported, perhaps this "give up
ability to pass an empty string as the default" approach may be the
only thing you could do.

Yeah, thanks for the explanations and suggestions.