{"thread":{"id":"44857","subject":"[PATCH 0/5] extend git-describe pattern matching","startedAt":"2017-01-12T00:17:31Z","lastAt":"2017-01-18T21:11:37Z","messageCount":25,"participants":["Jacob Keller","Johannes Schindelin","Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"309242","messageId":"20170112001721.2534-1-jacob.e.keller@intel.com","threadId":"44857","inReplyTo":null,"subject":"[PATCH 0/5] extend git-describe pattern matching","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2017-01-12T00:17:16Z","receivedAt":"2017-01-12T00:17:31Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nTeach git describe and git name-rev the ability to match multiple\npatterns inclusively. Additionally, teach these commands to also accept\nnegative patterns to discard any refs which match.\n\nThe pattern lists for positive and negative patterns are inclusive. This\nmeans that for the positive patterns, a reference will be considered as\nlong as it matches at least one of the match patterns. It need not match\nall given patterns. Additionally for negative patterns, we will not\nconsider any ref which matches any negative pattern, even if it matches\none of the positive patterns.\n\nTogether this allows the ability to express far more sets of tags than a\nsingle match pattern alone. It does not provide quite the same depth as\nwould teaching full regexp but it is simpler and easy enough to\nunderstand.\n\nThis is a re-send of a series from a month or so ago, I've since\nre-based this on next since it appears that it was not picked up before.\n\nJacob Keller (5):\n  doc: add documentation for OPT_STRING_LIST\n  name-rev: extend --refs to accept multiple patterns\n  name-rev: add support to discard refs by pattern match\n  describe: teach --match to accept multiple patterns\n  describe: teach describe negative pattern matches\n\n Documentation/git-describe.txt                | 13 ++++++-\n Documentation/git-name-rev.txt                | 11 +++++-\n Documentation/technical/api-parse-options.txt |  5 +++\n builtin/describe.c                            | 51 ++++++++++++++++++++++----\n builtin/name-rev.c                            | 53 +++++++++++++++++++++------\n t/t6007-rev-list-cherry-pick-file.sh          | 37 +++++++++++++++++++\n t/t6120-describe.sh                           | 27 ++++++++++++++\n 7 files changed, 176 insertions(+), 21 deletions(-)\n\n-- \n2.11.0.403.g196674b8396b\n\n"},{"id":"309243","messageId":"20170112001721.2534-6-jacob.e.keller@intel.com","threadId":"44857","inReplyTo":"20170112001721.2534-1-jacob.e.keller@intel.com","subject":"[PATCH 5/5] describe: teach describe negative pattern matches","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2017-01-12T00:17:21Z","receivedAt":"2017-01-12T00:17:35Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nTeach git-describe the `--discard` option which will allow specifying\na glob pattern of tags to ignore. This can be combined with the\n`--match` patterns to enable more flexibility in determining which tags\nto consider.\n\nFor example, suppose you wish to find the first official release tag\nthat contains a certain commit. If we assume that official release tags\nare of the form \"v*\" and pre-release candidates include \"*rc*\" in their\nname, we can now find the first tag that introduces commit abcdef via:\n\n  git describe --contains --match=\"v*\" --discard=\"*rc*\"\n\nAdd documentation and tests for this change.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n Documentation/git-describe.txt |  8 ++++++++\n builtin/describe.c             | 21 +++++++++++++++++++++\n t/t6120-describe.sh            |  8 ++++++++\n 3 files changed, 37 insertions(+)\n\ndiff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\nindex 7ad41e2f6ade..a89bbde207b2 100644\n--- a/Documentation/git-describe.txt\n+++ b/Documentation/git-describe.txt\n@@ -88,6 +88,14 @@ OPTIONS\n \tpatterns will be considered. Use `--no-match` to clear and reset the\n \tlist of patterns.\n \n+--discard <pattern>::\n+\tDo not consider tags matching the given `glob(7)` pattern, excluding\n+\tthe \"refs/tags/\" prefix. This can be used to narrow the tag space and\n+\tfind only tags matching some meaningful criteria. If given multiple\n+\ttimes, a list of patterns will be accumulated and tags matching any\n+\tof the patterns will be discarded. Use `--no-discard` to clear and\n+\treset the list of patterns.\n+\n --always::\n \tShow uniquely abbreviated commit object as fallback.\n \ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 5cc9e9abe798..c09288ee6321 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -29,6 +29,7 @@ static int max_candidates = 10;\n static struct hashmap names;\n static int have_util;\n static struct string_list patterns = STRING_LIST_INIT_NODUP;\n+static struct string_list discard_patterns = STRING_LIST_INIT_NODUP;\n static int always;\n static const char *dirty;\n \n@@ -130,6 +131,22 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi\n \t\treturn 0;\n \n \t/*\n+\t * If we're given discard patterns, first discard any tag which match\n+\t * any of the discard pattern.\n+\t */\n+\tif (discard_patterns.nr) {\n+\t\tstruct string_list_item *item;\n+\n+\t\tif (!is_tag)\n+\t\t\treturn 0;\n+\n+\t\tfor_each_string_list_item(item, &discard_patterns) {\n+\t\t\tif (!wildmatch(item->string, path + 10, 0, NULL))\n+\t\t\t\treturn 0;\n+\t\t}\n+\t}\n+\n+\t/*\n \t * If we're given patterns, accept only tags which match at least one\n \t * pattern.\n \t */\n@@ -421,6 +438,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t\t    N_(\"consider <n> most recent tags (default: 10)\")),\n \t\tOPT_STRING_LIST(0, \"match\", &patterns, N_(\"pattern\"),\n \t\t\t   N_(\"only consider tags matching <pattern>\")),\n+\t\tOPT_STRING_LIST(0, \"discard\", &discard_patterns, N_(\"pattern\"),\n+\t\t\t   N_(\"do not consider tags matching <pattern>\")),\n \t\tOPT_BOOL(0, \"always\",        &always,\n \t\t\tN_(\"show abbreviated commit object as fallback\")),\n \t\t{OPTION_STRING, 0, \"dirty\",  &dirty, N_(\"mark\"),\n@@ -458,6 +477,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t\targv_array_push(&args, \"--tags\");\n \t\t\tfor_each_string_list_item(item, &patterns)\n \t\t\t\targv_array_pushf(&args, \"--refs=refs/tags/%s\", item->string);\n+\t\t\tfor_each_string_list_item(item, &discard_patterns)\n+\t\t\t\targv_array_pushf(&args, \"--discard=refs/tags/%s\", item->string);\n \t\t}\n \t\tif (argc)\n \t\t\targv_array_pushv(&args, argv);\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 9e5db9b87a1f..4e4a9f2e5305 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -218,6 +218,14 @@ test_expect_success 'describe --contains and --match' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'describe --discard' '\n+\techo \"c~1\" >expect &&\n+\ttagged_commit=$(git rev-parse \"refs/tags/A^0\") &&\n+\ttest_must_fail git describe --contains --match=\"B\" $tagged_commit &&\n+\tgit describe --contains --match=\"?\" --discard=\"A\" $tagged_commit >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'describe --contains and --no-match' '\n \techo \"A^0\" >expect &&\n \ttagged_commit=$(git rev-parse \"refs/tags/A^0\") &&\n-- \n2.11.0.403.g196674b8396b\n\n"},{"id":"309244","messageId":"20170112001721.2534-5-jacob.e.keller@intel.com","threadId":"44857","inReplyTo":"20170112001721.2534-1-jacob.e.keller@intel.com","subject":"[PATCH 4/5] describe: teach --match to accept multiple patterns","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2017-01-12T00:17:20Z","receivedAt":"2017-01-12T00:17:39Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nTeach `--match` to be accepted multiple times, accumulating a list of\npatterns to match into a string list. Each pattern is inclusive, such\nthat a tag need only match one of the provided patterns to be\nconsidered for matching.\n\nThis extension is useful as it enables more flexibility in what tags\nmatch, and may avoid the need to run the describe command multiple\ntimes to get the same result.\n\nAdd tests and update the documentation for this change.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n Documentation/git-describe.txt |  5 ++++-\n builtin/describe.c             | 30 +++++++++++++++++++++++-------\n t/t6120-describe.sh            | 19 +++++++++++++++++++\n 3 files changed, 46 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt\nindex e4ac448ff565..7ad41e2f6ade 100644\n--- a/Documentation/git-describe.txt\n+++ b/Documentation/git-describe.txt\n@@ -83,7 +83,10 @@ OPTIONS\n --match <pattern>::\n \tOnly consider tags matching the given `glob(7)` pattern,\n \texcluding the \"refs/tags/\" prefix.  This can be used to avoid\n-\tleaking private tags from the repository.\n+\tleaking private tags from the repository. If given multiple times, a\n+\tlist of patterns will be accumulated, and tags matching any of the\n+\tpatterns will be considered. Use `--no-match` to clear and reset the\n+\tlist of patterns.\n \n --always::\n \tShow uniquely abbreviated commit object as fallback.\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 01490a157efc..5cc9e9abe798 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -28,7 +28,7 @@ static int abbrev = -1; /* unspecified */\n static int max_candidates = 10;\n static struct hashmap names;\n static int have_util;\n-static const char *pattern;\n+static struct string_list patterns = STRING_LIST_INIT_NODUP;\n static int always;\n static const char *dirty;\n \n@@ -129,9 +129,24 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi\n \tif (!all && !is_tag)\n \t\treturn 0;\n \n-\t/* Accept only tags that match the pattern, if given */\n-\tif (pattern && (!is_tag || wildmatch(pattern, path + 10, 0, NULL)))\n-\t\treturn 0;\n+\t/*\n+\t * If we're given patterns, accept only tags which match at least one\n+\t * pattern.\n+\t */\n+\tif (patterns.nr) {\n+\t\tstruct string_list_item *item;\n+\n+\t\tif (!is_tag)\n+\t\t\treturn 0;\n+\n+\t\tfor_each_string_list_item(item, &patterns) {\n+\t\t\tif (!wildmatch(item->string, path + 10, 0, NULL))\n+\t\t\t\tbreak;\n+\n+\t\t\t/* If we get here, no pattern matched. */\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n \n \t/* Is it annotated? */\n \tif (!peel_ref(path, peeled.hash)) {\n@@ -404,7 +419,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t\t    N_(\"only output exact matches\"), 0),\n \t\tOPT_INTEGER(0, \"candidates\", &max_candidates,\n \t\t\t    N_(\"consider <n> most recent tags (default: 10)\")),\n-\t\tOPT_STRING(0, \"match\",       &pattern, N_(\"pattern\"),\n+\t\tOPT_STRING_LIST(0, \"match\", &patterns, N_(\"pattern\"),\n \t\t\t   N_(\"only consider tags matching <pattern>\")),\n \t\tOPT_BOOL(0, \"always\",        &always,\n \t\t\tN_(\"show abbreviated commit object as fallback\")),\n@@ -430,6 +445,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"--long is incompatible with --abbrev=0\"));\n \n \tif (contains) {\n+\t\tstruct string_list_item *item;\n \t\tstruct argv_array args;\n \n \t\targv_array_init(&args);\n@@ -440,8 +456,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t\t\targv_array_push(&args, \"--always\");\n \t\tif (!all) {\n \t\t\targv_array_push(&args, \"--tags\");\n-\t\t\tif (pattern)\n-\t\t\t\targv_array_pushf(&args, \"--refs=refs/tags/%s\", pattern);\n+\t\t\tfor_each_string_list_item(item, &patterns)\n+\t\t\t\targv_array_pushf(&args, \"--refs=refs/tags/%s\", item->string);\n \t\t}\n \t\tif (argc)\n \t\t\targv_array_pushv(&args, argv);\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 85f269411cb3..9e5db9b87a1f 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -182,6 +182,10 @@ check_describe \"test2-lightweight-*\" --tags --match=\"test2-*\"\n \n check_describe \"test2-lightweight-*\" --long --tags --match=\"test2-*\" HEAD^\n \n+check_describe \"test1-lightweight-*\" --long --tags --match=\"test1-*\" --match=\"test2-*\" HEAD^\n+\n+check_describe \"test2-lightweight-*\" --long --tags --match=\"test1-*\" --no-match --match=\"test2-*\" HEAD^\n+\n test_expect_success 'name-rev with exact tags' '\n \techo A >expect &&\n \ttag_object=$(git rev-parse refs/tags/A) &&\n@@ -206,4 +210,19 @@ test_expect_success 'describe --contains with the exact tags' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'describe --contains and --match' '\n+\techo \"A^0\" >expect &&\n+\ttagged_commit=$(git rev-parse \"refs/tags/A^0\") &&\n+\ttest_must_fail git describe --contains --match=\"B\" $tagged_commit &&\n+\tgit describe --contains --match=\"B\" --match=\"A\" $tagged_commit >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'describe --contains and --no-match' '\n+\techo \"A^0\" >expect &&\n+\ttagged_commit=$(git rev-parse \"refs/tags/A^0\") &&\n+\tgit describe --contains --match=\"B\" --no-match $tagged_commit >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.11.0.403.g196674b8396b\n\n"},{"id":"309245","messageId":"20170112001721.2534-3-jacob.e.keller@intel.com","threadId":"44857","inReplyTo":"20170112001721.2534-1-jacob.e.keller@intel.com","subject":"[PATCH 2/5] name-rev: extend --refs to accept multiple patterns","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2017-01-12T00:17:18Z","receivedAt":"2017-01-12T00:17:45Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nTeach git name-rev to take a string list of patterns from --refs instead\nof only a single pattern. The list of patterns will be matched\ninclusively, such that a ref only needs to match one pattern to be\nincluded. If a ref will only be excluded if it does not match any of the\npatterns.\n\nAdd tests and documentation for this change.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n Documentation/git-name-rev.txt       |  4 +++-\n builtin/name-rev.c                   | 41 +++++++++++++++++++++++++-----------\n t/t6007-rev-list-cherry-pick-file.sh | 30 ++++++++++++++++++++++++++\n 3 files changed, 62 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt\nindex ca28fb8e2a07..7433627db12d 100644\n--- a/Documentation/git-name-rev.txt\n+++ b/Documentation/git-name-rev.txt\n@@ -26,7 +26,9 @@ OPTIONS\n \n --refs=<pattern>::\n \tOnly use refs whose names match a given shell pattern.  The pattern\n-\tcan be one of branch name, tag name or fully qualified ref name.\n+\tcan be one of branch name, tag name or fully qualified ref name. If\n+\tgiven multiple times, use refs whose names match any of the given shell\n+\tpatterns. Use `--no-refs` to clear any previous ref patterns given.\n \n --all::\n \tList all commits reachable from all refs\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex cd89d48b65e8..000a2a700ed3 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -108,7 +108,7 @@ static const char *name_ref_abbrev(const char *refname, int shorten_unambiguous)\n struct name_ref_data {\n \tint tags_only;\n \tint name_only;\n-\tconst char *ref_filter;\n+\tstruct string_list ref_filters;\n };\n \n static struct tip_table {\n@@ -150,16 +150,33 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \tif (data->tags_only && !starts_with(path, \"refs/tags/\"))\n \t\treturn 0;\n \n-\tif (data->ref_filter) {\n-\t\tswitch (subpath_matches(path, data->ref_filter)) {\n-\t\tcase -1: /* did not match */\n-\t\t\treturn 0;\n-\t\tcase 0:  /* matched fully */\n-\t\t\tbreak;\n-\t\tdefault: /* matched subpath */\n-\t\t\tcan_abbreviate_output = 1;\n-\t\t\tbreak;\n+\tif (data->ref_filters.nr) {\n+\t\tstruct string_list_item *item;\n+\t\tint matched = 0;\n+\n+\t\t/* See if any of the patterns match. */\n+\t\tfor_each_string_list_item(item, &data->ref_filters) {\n+\t\t\t/*\n+\t\t\t * We want to check every pattern even if we already\n+\t\t\t * found a match, just in case one of the later\n+\t\t\t * patterns could abbreviate the output.\n+\t\t\t */\n+\t\t\tswitch (subpath_matches(path, item->string)) {\n+\t\t\tcase -1: /* did not match */\n+\t\t\t\tbreak;\n+\t\t\tcase 0: /* matched fully */\n+\t\t\t\tmatched = 1;\n+\t\t\t\tbreak;\n+\t\t\tdefault: /* matched subpath */\n+\t\t\t\tmatched = 1;\n+\t\t\t\tcan_abbreviate_output = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n \t\t}\n+\n+\t\t/* If none of the patterns matched, stop now */\n+\t\tif (!matched)\n+\t\t\treturn 0;\n \t}\n \n \tadd_to_tip_table(oid->hash, path, can_abbreviate_output);\n@@ -306,11 +323,11 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n {\n \tstruct object_array revs = OBJECT_ARRAY_INIT;\n \tint all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;\n-\tstruct name_ref_data data = { 0, 0, NULL };\n+\tstruct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP };\n \tstruct option opts[] = {\n \t\tOPT_BOOL(0, \"name-only\", &data.name_only, N_(\"print only names (no SHA-1)\")),\n \t\tOPT_BOOL(0, \"tags\", &data.tags_only, N_(\"only use tags to name the commits\")),\n-\t\tOPT_STRING(0, \"refs\", &data.ref_filter, N_(\"pattern\"),\n+\t\tOPT_STRING_LIST(0, \"refs\", &data.ref_filters, N_(\"pattern\"),\n \t\t\t\t   N_(\"only use refs matching <pattern>\")),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL(0, \"all\", &all, N_(\"list all commits reachable from all refs\")),\ndiff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh\nindex 1408b608eb03..d072ec43b016 100755\n--- a/t/t6007-rev-list-cherry-pick-file.sh\n+++ b/t/t6007-rev-list-cherry-pick-file.sh\n@@ -99,6 +99,36 @@ test_expect_success '--cherry-pick bar does not come up empty (II)' '\n \ttest_cmp actual.named expect\n '\n \n+test_expect_success 'name-rev multiple --refs combine inclusive' '\n+\tgit rev-list --left-right --cherry-pick F...E -- bar > actual &&\n+\tgit name-rev --stdin --name-only --refs=\"*tags/F\" --refs=\"*tags/E\" \\\n+\t\t< actual > actual.named &&\n+\ttest_cmp actual.named expect\n+'\n+\n+cat >expect <<EOF\n+<tags/F\n+$(git rev-list --left-right --right-only --cherry-pick F...E -- bar)\n+EOF\n+\n+test_expect_success 'name-rev --refs excludes non-matched patterns' '\n+\tgit rev-list --left-right --cherry-pick F...E -- bar > actual &&\n+\tgit name-rev --stdin --name-only --refs=\"*tags/F\" \\\n+\t\t< actual > actual.named &&\n+\ttest_cmp actual.named expect\n+'\n+\n+cat >expect <<EOF\n+$(git rev-list --left-right --cherry-pick F...E -- bar)\n+EOF\n+\n+test_expect_success 'name-rev --no-refs clears the refs list' '\n+\tgit rev-list --left-right --cherry-pick F...E -- bar > actual &&\n+\tgit name-rev --stdin --name-only --refs=\"*tags/F\" --refs=\"*tags/E\" --no-refs --refs=\"*tags/G\" \\\n+\t\t< actual > actual.named &&\n+\ttest_cmp actual.named expect\n+'\n+\n cat >expect <<EOF\n +tags/F\n =tags/D\n-- \n2.11.0.403.g196674b8396b\n\n"},{"id":"309246","messageId":"20170112001721.2534-4-jacob.e.keller@intel.com","threadId":"44857","inReplyTo":"20170112001721.2534-1-jacob.e.keller@intel.com","subject":"[PATCH 3/5] name-rev: add support to discard refs by pattern match","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2017-01-12T00:17:19Z","receivedAt":"2017-01-12T00:17:48Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nExtend name-rev further to support matching refs by adding `--discard`\npatterns. These patterns will limit the scope of refs by discarding any\nref that matches at least one discard pattern. Checking the discard refs\nshall happen first, before checking the include --refs patterns. This\nwill allow more flexibility to matching certain kinds of references.\n\nAdd tests and update Documentation for this change.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n Documentation/git-name-rev.txt       |  7 +++++++\n builtin/name-rev.c                   | 14 +++++++++++++-\n t/t6007-rev-list-cherry-pick-file.sh |  7 +++++++\n 3 files changed, 27 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt\nindex 7433627db12d..9b46e5ea9aae 100644\n--- a/Documentation/git-name-rev.txt\n+++ b/Documentation/git-name-rev.txt\n@@ -30,6 +30,13 @@ OPTIONS\n \tgiven multiple times, use refs whose names match any of the given shell\n \tpatterns. Use `--no-refs` to clear any previous ref patterns given.\n \n+--discard=<pattern>::\n+\tDo not use any ref whose name matches a given shell pattern. The\n+\tpattern can be one of branch name, tag name or fully qualified ref\n+\tname. If given multiple times, discard refs that match any of the given\n+\tshell patterns. Use `--no-discards` to clear the list of discard\n+\tpatterns.\n+\n --all::\n \tList all commits reachable from all refs\n \ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex 000a2a700ed3..86479c17a7c9 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -109,6 +109,7 @@ struct name_ref_data {\n \tint tags_only;\n \tint name_only;\n \tstruct string_list ref_filters;\n+\tstruct string_list discard_filters;\n };\n \n static struct tip_table {\n@@ -150,6 +151,15 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo\n \tif (data->tags_only && !starts_with(path, \"refs/tags/\"))\n \t\treturn 0;\n \n+\tif (data->discard_filters.nr) {\n+\t\tstruct string_list_item *item;\n+\n+\t\tfor_each_string_list_item(item, &data->discard_filters) {\n+\t\t\tif (subpath_matches(path, item->string) >= 0)\n+\t\t\t\treturn 0;\n+\t\t}\n+\t}\n+\n \tif (data->ref_filters.nr) {\n \t\tstruct string_list_item *item;\n \t\tint matched = 0;\n@@ -323,12 +333,14 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n {\n \tstruct object_array revs = OBJECT_ARRAY_INIT;\n \tint all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;\n-\tstruct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP };\n+\tstruct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };\n \tstruct option opts[] = {\n \t\tOPT_BOOL(0, \"name-only\", &data.name_only, N_(\"print only names (no SHA-1)\")),\n \t\tOPT_BOOL(0, \"tags\", &data.tags_only, N_(\"only use tags to name the commits\")),\n \t\tOPT_STRING_LIST(0, \"refs\", &data.ref_filters, N_(\"pattern\"),\n \t\t\t\t   N_(\"only use refs matching <pattern>\")),\n+\t\tOPT_STRING_LIST(0, \"discard\", &data.discard_filters, N_(\"pattern\"),\n+\t\t\t\t   N_(\"ignore refs matching <pattern>\")),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL(0, \"all\", &all, N_(\"list all commits reachable from all refs\")),\n \t\tOPT_BOOL(0, \"stdin\", &transform_stdin, N_(\"read from stdin\")),\ndiff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh\nindex d072ec43b016..8a4c35f6ffee 100755\n--- a/t/t6007-rev-list-cherry-pick-file.sh\n+++ b/t/t6007-rev-list-cherry-pick-file.sh\n@@ -118,6 +118,13 @@ test_expect_success 'name-rev --refs excludes non-matched patterns' '\n \ttest_cmp actual.named expect\n '\n \n+test_expect_success 'name-rev --discard excludes matched patterns' '\n+\tgit rev-list --left-right --cherry-pick F...E -- bar > actual &&\n+\tgit name-rev --stdin --name-only --refs=\"*tags/*\" --discard=\"*E\" \\\n+\t\t< actual > actual.named &&\n+\ttest_cmp actual.named expect\n+'\n+\n cat >expect <<EOF\n $(git rev-list --left-right --cherry-pick F...E -- bar)\n EOF\n-- \n2.11.0.403.g196674b8396b\n\n"},{"id":"309247","messageId":"20170112001721.2534-2-jacob.e.keller@intel.com","threadId":"44857","inReplyTo":"20170112001721.2534-1-jacob.e.keller@intel.com","subject":"[PATCH 1/5] doc: add documentation for OPT_STRING_LIST","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2017-01-12T00:17:17Z","receivedAt":"2017-01-12T00:17:50Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nCommit c8ba16391655 (\"parse-options: add OPT_STRING_LIST helper\",\n2011-06-09) added the OPT_STRING_LIST as a way to accumulate a repeated\nlist of strings. However, this was not documented in the\napi-parse-options documentation. Add documentation now so that future\ndevelopers may learn of its existence.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n Documentation/technical/api-parse-options.txt | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt\nindex 27bd701c0d68..15e876e4c804 100644\n--- a/Documentation/technical/api-parse-options.txt\n+++ b/Documentation/technical/api-parse-options.txt\n@@ -168,6 +168,11 @@ There are some macros to easily define options:\n \tIntroduce an option with string argument.\n \tThe string argument is put into `str_var`.\n \n+`OPT_STRING_LIST(short, long, &list, arg_str, description)`::\n+\tIntroduce an option with a string argument. Repeated invocations\n+\taccumulate into a list of strings. Reset and clear the list with\n+\t`--no-option`.\n+\n `OPT_INTEGER(short, long, &int_var, description)`::\n \tIntroduce an option with integer argument.\n \tThe integer is put into `int_var`.\n-- \n2.11.0.403.g196674b8396b\n\n"},{"id":"309261","messageId":"alpine.DEB.2.20.1701121041450.3469@virtualbox","threadId":"44857","inReplyTo":"20170112001721.2534-6-jacob.e.keller@intel.com","subject":"Re: [PATCH 5/5] describe: teach describe negative pattern matches","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-12T09:42:44Z","receivedAt":"2017-01-12T09:43:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jake,\n\nOn Wed, 11 Jan 2017, Jacob Keller wrote:\n\n> From: Jacob Keller <jacob.keller@gmail.com>\n> \n> Teach git-describe the `--discard` option which will allow specifying\n> a glob pattern of tags to ignore.\n\nIMHO \"discard\" is the wrong word, it almost sounds as if the matching tags\nwould be *deleted*.\n\nMaybe `--exclude` or `--unmatch` instead?\n\nCiao,\nDscho\n"},{"id":"309262","messageId":"alpine.DEB.2.20.1701121044120.3469@virtualbox","threadId":"44857","inReplyTo":"20170112001721.2534-2-jacob.e.keller@intel.com","subject":"Re: [PATCH 1/5] doc: add documentation for OPT_STRING_LIST","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-12T09:47:08Z","receivedAt":"2017-01-12T09:54:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jake,\n\nOn Wed, 11 Jan 2017, Jacob Keller wrote:\n\n> diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt\n> index 27bd701c0d68..15e876e4c804 100644\n> --- a/Documentation/technical/api-parse-options.txt\n> +++ b/Documentation/technical/api-parse-options.txt\n> @@ -168,6 +168,11 @@ There are some macros to easily define options:\n>  \tIntroduce an option with string argument.\n>  \tThe string argument is put into `str_var`.\n>  \n> +`OPT_STRING_LIST(short, long, &list, arg_str, description)`::\n> +\tIntroduce an option with a string argument. Repeated invocations\n> +\taccumulate into a list of strings. Reset and clear the list with\n> +\t`--no-option`.\n\nOne suggestions: as the list parameter is not type-safe (apart from\nchecking that it can be cast to a `void *`), it would be good to mention\nin the documentation that `list` must be of type `struct string_list`.\n\nI was about to suggest that `--no-option` may be misleading, as the\ncommand-line option is not really called `--option` in almost all cases,\nbut I see that the rest of that document uses that convention to refer to\nthe negated option already...\n\nCiao,\nDscho\n"},{"id":"309263","messageId":"alpine.DEB.2.20.1701121049530.3469@virtualbox","threadId":"44857","inReplyTo":"20170112001721.2534-3-jacob.e.keller@intel.com","subject":"Re: [PATCH 2/5] name-rev: extend --refs to accept multiple patterns","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-12T09:56:12Z","receivedAt":"2017-01-12T09:56:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jake,\n\nOn Wed, 11 Jan 2017, Jacob Keller wrote:\n\n> diff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh\n> index 1408b608eb03..d072ec43b016 100755\n> --- a/t/t6007-rev-list-cherry-pick-file.sh\n> +++ b/t/t6007-rev-list-cherry-pick-file.sh\n> @@ -99,6 +99,36 @@ test_expect_success '--cherry-pick bar does not come up empty (II)' '\n>  \ttest_cmp actual.named expect\n>  '\n>  \n> +test_expect_success 'name-rev multiple --refs combine inclusive' '\n> +\tgit rev-list --left-right --cherry-pick F...E -- bar > actual &&\n\nOur current coding style seems to skip the space between `>` and `actual`\n(this applies to all redirections added in this patch).\n\n> +\tgit name-rev --stdin --name-only --refs=\"*tags/F\" --refs=\"*tags/E\" \\\n> +\t\t< actual > actual.named &&\n> +\ttest_cmp actual.named expect\n> +'\n> +\n> +cat >expect <<EOF\n> +<tags/F\n> +$(git rev-list --left-right --right-only --cherry-pick F...E -- bar)\n> +EOF\n\nIn the current revision of t6007, we seem to list the expected output\nexplicitly, i.e. *not* generating it dynamically.\n\nIf you *do* insist to generate the `expect` file dynamically, a better way\nwould be to include that generation in the `test_expect_success` code so\nthat errors in the call can be caught, too:\n\ntest_expect_success 'name-rev --refs excludes non-matched patterns' '\n\techo \"<tags/F\" >expect &&\n\tgit rev-list --left-right --right-only --cherry-pick F...E -- \\\n\t\tbar >>expect &&\n\t[...]\n\nHowever, if I was asked for my preference, I would suggest to specify the\n`expect` contents explicitly, to document the expectation as of time of\nwriting. The reason: I debugged my share of test breakages and these\ndynamically-generated `expect` files are the worst. When things break, you\nhave to dig *real* deep to figure out what is going wrong, as sometimes\nthe *generation of the `expect` file* regresses.\n\nCiao,\nDscho\n"},{"id":"309264","messageId":"alpine.DEB.2.20.1701121056470.3469@virtualbox","threadId":"44857","inReplyTo":"20170112001721.2534-4-jacob.e.keller@intel.com","subject":"Re: [PATCH 3/5] name-rev: add support to discard refs by pattern match","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-12T09:57:58Z","receivedAt":"2017-01-12T09:58:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jake,\n\nOn Wed, 11 Jan 2017, Jacob Keller wrote:\n\n> From: Jacob Keller <jacob.keller@gmail.com>\n> \n> Extend name-rev further to support matching refs by adding `--discard`\n> patterns.\n\nSame comment applies as for 5/5: `--exclude-refs` may be a better name\nthan `--discard`.\n\nCiao,\nDscho\n"},{"id":"309265","messageId":"alpine.DEB.2.20.1701121105070.3469@virtualbox","threadId":"44857","inReplyTo":"20170112001721.2534-1-jacob.e.keller@intel.com","subject":"Re: [PATCH 0/5] extend git-describe pattern matching","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-12T10:05:59Z","receivedAt":"2017-01-12T10:08:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jake,\n\nOn Wed, 11 Jan 2017, Jacob Keller wrote:\n\n> From: Jacob Keller <jacob.keller@gmail.com>\n> \n> Teach git describe and git name-rev the ability to match multiple\n> patterns inclusively. Additionally, teach these commands to also accept\n> negative patterns to discard any refs which match.\n\nI like the idea, and I think this patch series is already in a pretty good\nshape, offering a couple of comments as individual replies to the\nrespective patches.\n\nCiao,\nDscho\n"},{"id":"309272","messageId":"5f723a0d-623f-bf97-00de-29d430484fed@kdbg.org","threadId":"44857","inReplyTo":"20170112001721.2534-6-jacob.e.keller@intel.com","subject":"Re: [PATCH 5/5] describe: teach describe negative pattern matches","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-01-12T13:45:26Z","receivedAt":"2017-01-12T13:45:34Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12.01.2017 um 01:17 schrieb Jacob Keller:\n> From: Jacob Keller <jacob.keller@gmail.com>\n>\n> Teach git-describe the `--discard` option which will allow specifying\n> a glob pattern of tags to ignore. This can be combined with the\n> `--match` patterns to enable more flexibility in determining which tags\n> to consider.\n>\n> For example, suppose you wish to find the first official release tag\n> that contains a certain commit. If we assume that official release tags\n> are of the form \"v*\" and pre-release candidates include \"*rc*\" in their\n> name, we can now find the first tag that introduces commit abcdef via:\n>\n>   git describe --contains --match=\"v*\" --discard=\"*rc*\"\n\nI have a few dozen topic branches, many of them are work in progress and \nnamed wip/something. To see the completed branches, I routinely say\n\n     gitk --exclude=wip/* --branches\n\nthese days.\n\nIt would be great if you could provide the same user interface here. The \nexample in the commit message would then look like this:\n\n    git describe --contains --exclude=\"*rc*\" --match=\"v*\"\n\n(I'm not saying that you should add --branches, but that you should \nprefer --exclude over --discard. Also, the order of --exclude and \n--match would be important.)\n\n-- Hannes\n\n"},{"id":"309299","messageId":"xmqqlgughr7b.fsf@gitster.mtv.corp.google.com","threadId":"44857","inReplyTo":"alpine.DEB.2.20.1701121041450.3469@virtualbox","subject":"Re: [PATCH 5/5] describe: teach describe negative pattern matches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-12T22:02:48Z","receivedAt":"2017-01-12T22:02:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi Jake,\n>\n> On Wed, 11 Jan 2017, Jacob Keller wrote:\n>\n>> From: Jacob Keller <jacob.keller@gmail.com>\n>> \n>> Teach git-describe the `--discard` option which will allow specifying\n>> a glob pattern of tags to ignore.\n>\n> IMHO \"discard\" is the wrong word, it almost sounds as if the matching tags\n> would be *deleted*.\n>\n> Maybe `--exclude` or `--unmatch` instead?\n\nYeah, as j6t mentions, I think --exclude would be a good choice.\n"},{"id":"309336","messageId":"CA+P7+xoJfiqHy28z8ynonYR3ujoCUB4_diGJHP3XSp=iWpTB5A@mail.gmail.com","threadId":"44857","inReplyTo":"alpine.DEB.2.20.1701121044120.3469@virtualbox","subject":"Re: [PATCH 1/5] doc: add documentation for OPT_STRING_LIST","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-01-13T00:51:59Z","receivedAt":"2017-01-13T00:52:33Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Jan 12, 2017 at 1:47 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi Jake,\n>\n> On Wed, 11 Jan 2017, Jacob Keller wrote:\n>\n>> diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt\n>> index 27bd701c0d68..15e876e4c804 100644\n>> --- a/Documentation/technical/api-parse-options.txt\n>> +++ b/Documentation/technical/api-parse-options.txt\n>> @@ -168,6 +168,11 @@ There are some macros to easily define options:\n>>       Introduce an option with string argument.\n>>       The string argument is put into `str_var`.\n>>\n>> +`OPT_STRING_LIST(short, long, &list, arg_str, description)`::\n>> +     Introduce an option with a string argument. Repeated invocations\n>> +     accumulate into a list of strings. Reset and clear the list with\n>> +     `--no-option`.\n>\n> One suggestions: as the list parameter is not type-safe (apart from\n> checking that it can be cast to a `void *`), it would be good to mention\n> in the documentation that `list` must be of type `struct string_list`.\n>\n\nOk.\n\n> I was about to suggest that `--no-option` may be misleading, as the\n> command-line option is not really called `--option` in almost all cases,\n> but I see that the rest of that document uses that convention to refer to\n> the negated option already...\n\nAlso, I am merely documenting what already existed, since the original\ncommit failed to add documentation. I don't know if we could make a\nbetter implementation, but it certainly would be a behavior change for\nthe current users.\n\nThanks,\nJake\n\n>\n> Ciao,\n> Dscho\n"},{"id":"309337","messageId":"CA+P7+xpHQ7AobJNXTnHs1jf8Y67PJYqoR3yONFBb4k_=63CH5g@mail.gmail.com","threadId":"44857","inReplyTo":"alpine.DEB.2.20.1701121049530.3469@virtualbox","subject":"Re: [PATCH 2/5] name-rev: extend --refs to accept multiple patterns","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-01-13T00:56:11Z","receivedAt":"2017-01-13T00:56:38Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Jan 12, 2017 at 1:56 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi Jake,\n>\n> On Wed, 11 Jan 2017, Jacob Keller wrote:\n>\n>> diff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh\n>> index 1408b608eb03..d072ec43b016 100755\n>> --- a/t/t6007-rev-list-cherry-pick-file.sh\n>> +++ b/t/t6007-rev-list-cherry-pick-file.sh\n>> @@ -99,6 +99,36 @@ test_expect_success '--cherry-pick bar does not come up empty (II)' '\n>>       test_cmp actual.named expect\n>>  '\n>>\n>> +test_expect_success 'name-rev multiple --refs combine inclusive' '\n>> +     git rev-list --left-right --cherry-pick F...E -- bar > actual &&\n>\n> Our current coding style seems to skip the space between `>` and `actual`\n> (this applies to all redirections added in this patch).\n>\n\nRight that's easy to fix.\n\n>> +     git name-rev --stdin --name-only --refs=\"*tags/F\" --refs=\"*tags/E\" \\\n>> +             < actual > actual.named &&\n>> +     test_cmp actual.named expect\n>> +'\n>> +\n>> +cat >expect <<EOF\n>> +<tags/F\n>> +$(git rev-list --left-right --right-only --cherry-pick F...E -- bar)\n>> +EOF\n>\n> In the current revision of t6007, we seem to list the expected output\n> explicitly, i.e. *not* generating it dynamically.\n>\n> If you *do* insist to generate the `expect` file dynamically, a better way\n> would be to include that generation in the `test_expect_success` code so\n> that errors in the call can be caught, too:\n>\n\nThe main reason I don't like static expecations is that often other\ntests inserted before or after my test now have to know what to put\nhere. Specifically, the problem is that we expect the output not to be\nnamed, but actually to be sha1 hex output. This seems really brittle\nfor a test.\n\n> test_expect_success 'name-rev --refs excludes non-matched patterns' '\n>         echo \"<tags/F\" >expect &&\n>         git rev-list --left-right --right-only --cherry-pick F...E -- \\\n>                 bar >>expect &&\n>         [...]\n>\n> However, if I was asked for my preference, I would suggest to specify the\n> `expect` contents explicitly, to document the expectation as of time of\n> writing. The reason: I debugged my share of test breakages and these\n> dynamically-generated `expect` files are the worst. When things break, you\n> have to dig *real* deep to figure out what is going wrong, as sometimes\n> the *generation of the `expect` file* regresses.\n>\n\nDo you have a better suggestion for how to check the expect vs actual\noutput ignoring the raw hex data that would be there otherwise? What I\nwant to avoid is a brittle test that depends precisely on actions of\nprior tests in generating commits and tags. Specifically in this case,\nwe're going to end up with the sha1 hex ID of the commit in the\nexpected value.. hard-coding that feels wrong.\n\nThanks,\nJake\n\n> Ciao,\n> Dscho\n"},{"id":"309338","messageId":"CA+P7+xp_23dfGUmuT3fTMOrBOaMjvZpu0ax7A3L8RfQDK4hLbg@mail.gmail.com","threadId":"44857","inReplyTo":"alpine.DEB.2.20.1701121056470.3469@virtualbox","subject":"Re: [PATCH 3/5] name-rev: add support to discard refs by pattern match","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-01-13T00:56:41Z","receivedAt":"2017-01-13T00:57:08Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Jan 12, 2017 at 1:57 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi Jake,\n>\n> On Wed, 11 Jan 2017, Jacob Keller wrote:\n>\n>> From: Jacob Keller <jacob.keller@gmail.com>\n>>\n>> Extend name-rev further to support matching refs by adding `--discard`\n>> patterns.\n>\n> Same comment applies as for 5/5: `--exclude-refs` may be a better name\n> than `--discard`.\n>\n\nI agree, will change.\n\nThanks,\nJake\n\n> Ciao,\n> Dscho\n"},{"id":"309339","messageId":"CA+P7+xrmAmCPOzuaKcm+WxceXnowkM4gKz05tSpdC=CDwpCEug@mail.gmail.com","threadId":"44857","inReplyTo":"5f723a0d-623f-bf97-00de-29d430484fed@kdbg.org","subject":"Re: [PATCH 5/5] describe: teach describe negative pattern matches","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-01-13T00:59:46Z","receivedAt":"2017-01-13T01:00:13Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Jan 12, 2017 at 5:45 AM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 12.01.2017 um 01:17 schrieb Jacob Keller:\n>>\n>> From: Jacob Keller <jacob.keller@gmail.com>\n>>\n>> Teach git-describe the `--discard` option which will allow specifying\n>> a glob pattern of tags to ignore. This can be combined with the\n>> `--match` patterns to enable more flexibility in determining which tags\n>> to consider.\n>>\n>> For example, suppose you wish to find the first official release tag\n>> that contains a certain commit. If we assume that official release tags\n>> are of the form \"v*\" and pre-release candidates include \"*rc*\" in their\n>> name, we can now find the first tag that introduces commit abcdef via:\n>>\n>>   git describe --contains --match=\"v*\" --discard=\"*rc*\"\n>\n>\n> I have a few dozen topic branches, many of them are work in progress and\n> named wip/something. To see the completed branches, I routinely say\n>\n>     gitk --exclude=wip/* --branches\n>\n> these days.\n>\n> It would be great if you could provide the same user interface here. The\n> example in the commit message would then look like this:\n>\n>    git describe --contains --exclude=\"*rc*\" --match=\"v*\"\n>\n> (I'm not saying that you should add --branches, but that you should prefer\n> --exclude over --discard. Also, the order of --exclude and --match would be\n> important.)\n\nI think that --exclude makes sense, but the current implementation\ndoes not differentiate ordering, since both are merely accumulated\ninto string_lists and then matched together. I'm not sure how order\nwould impact things here? In the current implementation, if something\nis excluded and matched, it will be excluded. That is, exclusion\npatterns take precedence over match patterns. I think this makes the\nmost sense semantically.\n\nThanks,\nJake\n\n>\n> -- Hannes\n>\n"},{"id":"309344","messageId":"5c8401ef-9609-f235-9228-be980a13edf1@kdbg.org","threadId":"44857","inReplyTo":"CA+P7+xrmAmCPOzuaKcm+WxceXnowkM4gKz05tSpdC=CDwpCEug@mail.gmail.com","subject":"Re: [PATCH 5/5] describe: teach describe negative pattern matches","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-01-13T06:43:11Z","receivedAt":"2017-01-13T06:43:22Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 13.01.2017 um 01:59 schrieb Jacob Keller:\n> I think that --exclude makes sense, but the current implementation\n> does not differentiate ordering, since both are merely accumulated\n> into string_lists and then matched together. I'm not sure how order\n> would impact things here? In the current implementation, if something\n> is excluded and matched, it will be excluded. That is, exclusion\n> patterns take precedence over match patterns. I think this makes the\n> most sense semantically.\n\nWhen you write\n\n   git log --exclude=wip/* --branches --remotes\n\n--exclude applies only to --branches, not to --remotes.\n\n  When you write\n\n   git log --branches --exclude=origin/* --remotes\n\n--exclude=origin/* applies only to --remotes, but not to --branches.\n\n-- Hannes\n\n"},{"id":"309345","messageId":"CA+P7+xq1LMkRG_aSyamrsPUQE+rDv4A9Qd19tDMgx-_a5OHsqQ@mail.gmail.com","threadId":"44857","inReplyTo":"5c8401ef-9609-f235-9228-be980a13edf1@kdbg.org","subject":"Re: [PATCH 5/5] describe: teach describe negative pattern matches","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-01-13T06:57:28Z","receivedAt":"2017-01-13T06:58:16Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Jan 12, 2017 at 10:43 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 13.01.2017 um 01:59 schrieb Jacob Keller:\n>>\n>> I think that --exclude makes sense, but the current implementation\n>> does not differentiate ordering, since both are merely accumulated\n>> into string_lists and then matched together. I'm not sure how order\n>> would impact things here? In the current implementation, if something\n>> is excluded and matched, it will be excluded. That is, exclusion\n>> patterns take precedence over match patterns. I think this makes the\n>> most sense semantically.\n>\n>\n> When you write\n>\n>   git log --exclude=wip/* --branches --remotes\n>\n> --exclude applies only to --branches, not to --remotes.\n>\n>  When you write\n>\n>   git log --branches --exclude=origin/* --remotes\n>\n> --exclude=origin/* applies only to --remotes, but not to --branches.\n>\n> -- Hannes\n>\n\nWell for describe I don't think the order matters.\n\nThanks,\nJake\n"},{"id":"309384","messageId":"xmqqpojqhk3i.fsf@gitster.mtv.corp.google.com","threadId":"44857","inReplyTo":"20170112001721.2534-1-jacob.e.keller@intel.com","subject":"Re: [PATCH 0/5] extend git-describe pattern matching","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-13T18:48:33Z","receivedAt":"2017-01-13T18:49:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n> From: Jacob Keller <jacob.keller@gmail.com>\n>\n> Teach git describe and git name-rev the ability to match multiple\n> patterns inclusively. Additionally, teach these commands to also accept\n> negative patterns to discard any refs which match.\n\nYou made quick responses to reviews with \"will change\", so I am not\nqueuing this round to my tree; please don't mistake that as my\nindifference or opposition to the topic.  This sounds like a good\nthing.\n\nAs to the semantics of mixing positives and negatives, I would\nrecommend this to follow how positive and negative pathspecs mix.\nIIRC we chose to use the most simple and easy to explain option,\ni.e. a thing must match at least one of the positives and must not\nmatch any of the negatives to be considered a match.\n\n\n"},{"id":"309396","messageId":"CA+P7+xoR7oOEg-WW4AqWNYfn4f2W4vwxFLyxc+buzAi7h2xfyw@mail.gmail.com","threadId":"44857","inReplyTo":"xmqqpojqhk3i.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/5] extend git-describe pattern matching","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-01-13T20:41:44Z","receivedAt":"2017-01-13T20:42:36Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Jan 13, 2017 at 10:48 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jacob Keller <jacob.e.keller@intel.com> writes:\n>\n>> From: Jacob Keller <jacob.keller@gmail.com>\n>>\n>> Teach git describe and git name-rev the ability to match multiple\n>> patterns inclusively. Additionally, teach these commands to also accept\n>> negative patterns to discard any refs which match.\n>\n> You made quick responses to reviews with \"will change\", so I am not\n> queuing this round to my tree; please don't mistake that as my\n> indifference or opposition to the topic.  This sounds like a good\n> thing.\n\nPerfect. I will probably take a few days till I am back at a computer\nand can do this, but I will be submitting with the suggested changes\nsoon.\n\n>\n> As to the semantics of mixing positives and negatives, I would\n> recommend this to follow how positive and negative pathspecs mix.\n> IIRC we chose to use the most simple and easy to explain option,\n> i.e. a thing must match at least one of the positives and must not\n> match any of the negatives to be considered a match.\n>\n>\n\nThat is the current implementation, so I will stick with it. It's the\nsimplest, and easiest to implement.\n\nThanks,\nJake\n"},{"id":"309399","messageId":"97d4105c-0fa6-41c5-e456-30cebd93dc36@kdbg.org","threadId":"44857","inReplyTo":"CA+P7+xq1LMkRG_aSyamrsPUQE+rDv4A9Qd19tDMgx-_a5OHsqQ@mail.gmail.com","subject":"Re: [PATCH 5/5] describe: teach describe negative pattern matches","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-01-13T21:31:11Z","receivedAt":"2017-01-13T21:31:20Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 13.01.2017 um 07:57 schrieb Jacob Keller:\n> On Thu, Jan 12, 2017 at 10:43 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n>>  When you write\n>>\n>>   git log --branches --exclude=origin/* --remotes\n>>\n>> --exclude=origin/* applies only to --remotes, but not to --branches.\n>\n> Well for describe I don't think the order matters.\n\nThat is certainly true today. But I would value consistency more. We \nwould lose it if some time in the future 'describe' accepts --branches \nand --remotes in addition to --tags and --all.\n\n-- Hannes\n\n"},{"id":"309573","messageId":"CA+P7+xqFHG52Xo8ncUwa3owDn3OOz+rr3ZaGwfcUDCiXJmh80Q@mail.gmail.com","threadId":"44857","inReplyTo":"97d4105c-0fa6-41c5-e456-30cebd93dc36@kdbg.org","subject":"Re: [PATCH 5/5] describe: teach describe negative pattern matches","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-01-17T23:31:16Z","receivedAt":"2017-01-17T23:32:50Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Jan 13, 2017 at 1:31 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 13.01.2017 um 07:57 schrieb Jacob Keller:\n>>\n>> On Thu, Jan 12, 2017 at 10:43 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n>>>\n>>>  When you write\n>>>\n>>>   git log --branches --exclude=origin/* --remotes\n>>>\n>>> --exclude=origin/* applies only to --remotes, but not to --branches.\n>>\n>>\n>> Well for describe I don't think the order matters.\n>\n>\n> That is certainly true today. But I would value consistency more. We would\n> lose it if some time in the future 'describe' accepts --branches and\n> --remotes in addition to --tags and --all.\n>\n> -- Hannes\n>\n\nI am not sure that the interface for git-log and git-describe are\nsimilar enough to make this distinction work. --match already seems to\nimply that it only works on refs in refs/tags, as it says it considers\nglobs matching excluding the \"refs/tags\" prefix.\n\nIn git-describe, we already have \"--tags\" and \"--all\" but they are\nmutually exclusive. We don't support using more than one at once, and\nI'm not really convinced that describe will ever support more than one\nat a time. Additionally, match already doesn't respect order.\n\nThanks,\nJake\n"},{"id":"309623","messageId":"alpine.DEB.2.20.1701181340530.3469@virtualbox","threadId":"44857","inReplyTo":"CA+P7+xqFHG52Xo8ncUwa3owDn3OOz+rr3ZaGwfcUDCiXJmh80Q@mail.gmail.com","subject":"Re: [PATCH 5/5] describe: teach describe negative pattern matches","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-18T12:44:03Z","receivedAt":"2017-01-18T12:44:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jake,\n\nOn Tue, 17 Jan 2017, Jacob Keller wrote:\n\n> On Fri, Jan 13, 2017 at 1:31 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n> > Am 13.01.2017 um 07:57 schrieb Jacob Keller:\n> >>\n> >> On Thu, Jan 12, 2017 at 10:43 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n> >>>\n> >>>  When you write\n> >>>\n> >>>   git log --branches --exclude=origin/* --remotes\n> >>>\n> >>> --exclude=origin/* applies only to --remotes, but not to --branches.\n> >>\n> >>\n> >> Well for describe I don't think the order matters.\n> >\n> >\n> > That is certainly true today. But I would value consistency more. We would\n> > lose it if some time in the future 'describe' accepts --branches and\n> > --remotes in addition to --tags and --all.\n> >\n> > -- Hannes\n> >\n> \n> I am not sure that the interface for git-log and git-describe are\n> similar enough to make this distinction work. --match already seems to\n> imply that it only works on refs in refs/tags, as it says it considers\n> globs matching excluding the \"refs/tags\" prefix.\n> \n> In git-describe, we already have \"--tags\" and \"--all\" but they are\n> mutually exclusive. We don't support using more than one at once, and\n> I'm not really convinced that describe will ever support more than one\n> at a time. Additionally, match already doesn't respect order.\n\nI agree that it would keep the code much simpler if you kept the order\n\"exclude before include\".\n\nHowever, should you decide to look into making the logic dependent on the\norder in which the flags were specified in the command-line, we do have a\ndata structure for such a beast: we use it in gitignore and in\nsparse-checkout, it is called struct exclude_list.\n\nJust some food for thought,\nJohannes\n"},{"id":"309670","messageId":"CA+P7+xp6c_3gTWiSrZHuXrpeZxYPguYBaBOT+uwKwATx4+1npw@mail.gmail.com","threadId":"44857","inReplyTo":"alpine.DEB.2.20.1701181340530.3469@virtualbox","subject":"Re: [PATCH 5/5] describe: teach describe negative pattern matches","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-01-18T21:04:51Z","receivedAt":"2017-01-18T21:11:37Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Jan 18, 2017 at 4:44 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi Jake,\n>\n> On Tue, 17 Jan 2017, Jacob Keller wrote:\n>\n>> On Fri, Jan 13, 2017 at 1:31 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n>> > Am 13.01.2017 um 07:57 schrieb Jacob Keller:\n>> >>\n>> >> On Thu, Jan 12, 2017 at 10:43 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n>> >>>\n>> >>>  When you write\n>> >>>\n>> >>>   git log --branches --exclude=origin/* --remotes\n>> >>>\n>> >>> --exclude=origin/* applies only to --remotes, but not to --branches.\n>> >>\n>> >>\n>> >> Well for describe I don't think the order matters.\n>> >\n>> >\n>> > That is certainly true today. But I would value consistency more. We would\n>> > lose it if some time in the future 'describe' accepts --branches and\n>> > --remotes in addition to --tags and --all.\n>> >\n>> > -- Hannes\n>> >\n>>\n>> I am not sure that the interface for git-log and git-describe are\n>> similar enough to make this distinction work. --match already seems to\n>> imply that it only works on refs in refs/tags, as it says it considers\n>> globs matching excluding the \"refs/tags\" prefix.\n>>\n>> In git-describe, we already have \"--tags\" and \"--all\" but they are\n>> mutually exclusive. We don't support using more than one at once, and\n>> I'm not really convinced that describe will ever support more than one\n>> at a time. Additionally, match already doesn't respect order.\n>\n> I agree that it would keep the code much simpler if you kept the order\n> \"exclude before include\".\n>\n> However, should you decide to look into making the logic dependent on the\n> order in which the flags were specified in the command-line, we do have a\n> data structure for such a beast: we use it in gitignore and in\n> sparse-checkout, it is called struct exclude_list.\n>\n> Just some food for thought,\n> Johannes\n\nThat might help make it easier to go this route. I'll take a look.\n\nThanks,\nJake\n"}]}