Re: [PATCH 3/3 v5] tag: support configuring --sort via .gitconfig
- From
Jeff King <peff@peff.net>
- Date
- Jul 11, 2014, 17:46 UTC
- Message-ID
- <20140711174628.GC7856@sigill.intra.peff.net>
- In-Reply-To
- <1405099447-27555-3-git-send-email-jacob.e.keller@intel.com>
On Fri, Jul 11, 2014 at 10:24:07AM -0700, Jacob Keller wrote:
> Updated to include changes due to Junio's feedback. This has not resolved > whether we should fail on a configuration error or simply warn. It appears that > we actually seem to error out more than warn, so I am unsure what the correct > action is here.
Yeah, we're quite inconsistent there. In some cases we silently ignore something unknown (e.g., a color.diff.* slot that we do not understand), but in most cases if it is a config key we understand but a value we do not, we complain and die.
It's probably user-unfriendly to be silent for those cases, though. The user gets no feedback on why their config value is doing nothing.
I tend to think that warning is not much better than erroring out. It is helpful if you are running a single-shot of an old version (which is something that I do a lot when testing old versions), but would quickly become irritating if you were actually using an old version of git day-to-day.
I dunno. Maybe it is worth making life easier for people in the former category.
Show 19 quoted lines
> +static int parse_sort_string(const char *arg, int *sort)
> +{
> + int type = 0, flags = 0;
> +
> + if (skip_prefix(arg, "-", &arg))
> + flags |= REVERSE_SORT;
> +
> + if (skip_prefix(arg, "version:", &arg) || skip_prefix(arg, "v:", &arg))
> + type = VERCMP_SORT;
> + else
> + type = STRCMP_SORT;
> +
> + if (strcmp(arg, "refname"))
> + return error(_("unsupported sort specification %s"), arg);
> +
> + *sort = (type | flags);
> +
> + return 0;
> +}Regardless of how we handle the error, I think this version that assembles the final bitfield at the end is easier to read than the original.
-Peff