"Shawn O. Pearce" <spearce@xxxxxxxxxxx> writes:
> As a porcelain author I'm finding it difficult to keep track of
> what features I can use in git-gui. Newer versions of Git have
> newer capabilities but they don't always immediately get newer
> version numbers that I can easily test for.
Two and half comments, and a discussion.
> +static const char *supported_features[] = {
> + "blame-ignore-whitespace",
> + "list-features",
> +};
> +
> /* most GUI terminals set COLUMNS (although some don't export it) */
> static int term_columns(void)
> {
> @@ -190,10 +195,78 @@ void help_unknown_cmd(const char *cmd)
> exit(1);
> }
>
> +static int is_feature_name_sane(const char *a)
> +{
> + if (!*a || *a == '-')
> + return 0;
> + for (; *a; a++) {
> + if (! ((*a >= 'a' && *a <= 'z')
> + || (*a >= '0' && *a <= '9')
> + || *a == '-'))
> + return 0;
> + }
> + return 1;
> +}
> +
> +static int cmp_feature(const void *a_, const void *b_)
> +{
> + const char *a = *((const char **)a_);
> + const char *b = *((const char **)b_);
> + return strcmp(a, b);
> +}
> +
> +static void list_features()
> +{
> + unsigned cnt = ARRAY_SIZE(supported_features);
> + unsigned i;
> +
> + qsort(supported_features, cnt,
> + sizeof(supported_features[0]), cmp_feature);
> ...
> +}
Unless we are talking about dynamically extensible feature list
(eh, dll, anybody?), it might be easier to keep (1) the list
sorted, and (2) free of insane feature name in
supported_features[] array at the source level. Then you can
lose that is_feature_name_sane() function.
> +static int supports_feature(const char *the_feature)
> +{
> + unsigned cnt = ARRAY_SIZE(supported_features);
> + unsigned i;
> +
> + for (i = 0; i < cnt; i++) {
> + if (!strcmp(supported_features[i], the_feature))
> + return 0;
> + }
> + return 1;
> +}
And you can perform a bsearch here. instead of linear.
> +test_expect_failure \
> + 'feature "THISNEVERWILLBEAGITFEATURE" is not supported' \
> + 'git version --supports-feature=THISNEVERWILLBEAGITFEATURE'
I would expect that THISNEW... will get complaint saying "That
is not a valid feature name, as it has uppercase", from a
version that has is_feature_name_sane() function.
I suspect that this patch is meant for my 'maint' (and
1.5.2.3). Or is it for my 'master'? What's your plan to handle
transition?
For example, if this appears on 1.5.2.3, then
supported_features[] should not have blame-ignore-whitespace,
unless we are talking about cherry-picking, and I honestly do
not think "blame -w" deserves to go to the maintenance only
series. On the other hand, --list-features could go to 'maint'
under 'future prooofing' category, I guess.
If this is meant to be only for 1.5.3 and later, then you know
that "blame -w" is available as well, so the fact you can do
"git version --list-features" alone tells you that you can use
"blame -w", among other many things, such as "diff -C -C"
instead of --find-copies-harder.
Where does the above discussion lead us? It essentially means,
in either case, "blame-ignore-whitespace" should not be in that
supported_features[] array.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
