{"thread":{"id":"51952","subject":"[PATCH 0/1] gitignore: ignore comments on the same line as a pattern","startedAt":"2019-10-02T03:13:17Z","lastAt":"2019-10-02T07:02:56Z","messageCount":4,"participants":["Chris Zehner via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"383238","messageId":"pull.370.git.gitgitgadget@gmail.com","threadId":"51952","inReplyTo":null,"subject":"[PATCH 0/1] gitignore: ignore comments on the same line as a pattern","fromName":"Chris Zehner via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-02T03:13:12Z","receivedAt":"2019-10-02T03:13:17Z","isPatch":true,"sender":{"key":"name:Chris Zehner","avatar":null},"body":"Why make this change?\n=====================\n\nAdd the ability to use # to put comments after ignore patterns. This is\nuseful for documenting the reason for particular ignore patterns inclusion\nand structure.\n\nRight now a common convention in .gitignore files is to group patterns by\nsimilarity, using new lines beginning with one or more # characters as\nheadings to explain these groupings. This works well when clarifying why\nbroad classes of things are ignored, e.g. # build artifacts followed by\nseveral patterns.\n\nWhen leaving comments about a particular pattern it can be difficult to\ndistinguish comments about a single patterns from comments used for file\norganization.\n\nComments left after a pattern are unambiguously related to that line, and\nthat line only.\n\nWhat should this change do?\n===========================\n\nThe entirety of a string after a non-escaped #, including the #, is removed\nfrom the pattern in a .gitignore file.\n\nWhy make the change this way?\n=============================\n\nI don't normally write C, so I probably overlooked more idiomatic ways to do\nthis. This is done similarly to the way trim_trailing_spaces removes\nextraneous spaces from patterns.\n\nPotentially this change could be combined with the existing code for\ntrim_trailing_spaces, but the logic is slightly different and it seems to me\nthat the difference in naming aids readability by making it clear what the\nresponsibilities are for each function.\n\nHow can we test this change works?\n==================================\n\nThat's one area I'd like help with, please.\n\nTest cases:\n\n/pattern/to/match\n# Existing comment\n/pattern/with/comment # This comment is ignored\n/pattern/with/\\#octothorpe # \\#octothorpe is ignored\n\nI wasn't sure where the correct place to add these would be, I didn't see\n(and potentially overlooked) any tests in /t/* that cover this\nfunctionality. Would someone be willing to provide a pointer to the correct\nplace to add these tests?\n\nSigned-off-by: Chris Zehner cbzehner@gmail.com [cbzehner@gmail.com]\n\nChris Zehner (1):\n  gitignore: ignore comments on the same line as a pattern\n\n Documentation/gitignore.txt |  8 ++++++--\n dir.c                       | 33 +++++++++++++++++++++++++++++++++\n 2 files changed, 39 insertions(+), 2 deletions(-)\n\n\nbase-commit: bc12974a897308fd3254cf0cc90319078fe45eea\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-370%2Fcbzehner%2Fgitignore-pattern-comment-oneline-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-370/cbzehner/gitignore-pattern-comment-oneline-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/370\n-- \ngitgitgadget\n"},{"id":"383239","messageId":"e448214e87b46dc649781b10727043bc390d9aab.1569985991.git.gitgitgadget@gmail.com","threadId":"51952","inReplyTo":"pull.370.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] gitignore: ignore comments on the same line as a pattern","fromName":"Chris Zehner via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-02T03:13:13Z","receivedAt":"2019-10-02T03:13:17Z","isPatch":true,"sender":{"key":"name:Chris Zehner","avatar":null},"body":"From: Chris Zehner <cbzehner@gmail.com>\n\nSigned-off-by: Chris Zehner <cbzehner@gmail.com>\n---\n Documentation/gitignore.txt |  8 ++++++--\n dir.c                       | 33 +++++++++++++++++++++++++++++++++\n 2 files changed, 39 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt\nindex d47b1ae296..1778cf1d38 100644\n--- a/Documentation/gitignore.txt\n+++ b/Documentation/gitignore.txt\n@@ -74,8 +74,12 @@ PATTERN FORMAT\n    for readability.\n \n  - A line starting with # serves as a comment.\n-   Put a backslash (\"`\\`\") in front of the first hash for patterns\n-   that begin with a hash.\n+   Put a backslash (\"`\\`\") in front of each hash for patterns\n+   containing a hash.\n+\n+ - A # after a pattern will be treated as the start of a comment.\n+   Put a backslash (\"`\\`\") in front of each hash for patterns\n+   containing a hash.\n \n  - Trailing spaces are ignored unless they are quoted with backslash\n    (\"`\\`\").\ndiff --git a/dir.c b/dir.c\nindex cab9c2a458..aeefe142bc 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -658,6 +658,38 @@ void clear_pattern_list(struct pattern_list *pl)\n \tmemset(pl, 0, sizeof(*pl));\n }\n \n+static void trim_trailing_comments(char *buf)\n+{\n+\tchar *p, *last_hash = NULL;\n+\tint escape_seq = 0;\n+\n+\tfor (p = buf; *p; p++)\n+\t{\n+\t\tif (!*p)\n+\t\t\treturn;\n+\t\tswitch (*p) {\n+\t\tcase '#':\n+\t\t\tif (escape_seq)\n+\t\t\t{\n+\t\t\t\tescape_seq = 0;\n+\t\t\t\tp++;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tif (!last_hash)\n+\t\t\t\tlast_hash = p;\n+\t\t\tbreak;\n+\t\tcase '\\\\':\n+\t\t\tescape_seq = 1;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\tescape_seq = 0;\n+\t\t}\n+\t}\n+\n+\tif (last_hash)\n+\t\t*last_hash = '\\0';\n+}\n+\n static void trim_trailing_spaces(char *buf)\n {\n \tchar *p, *last_space = NULL;\n@@ -859,6 +891,7 @@ static int add_patterns_from_buffer(char *buf, size_t size,\n \t\tif (buf[i] == '\\n') {\n \t\t\tif (entry != buf + i && entry[0] != '#') {\n \t\t\t\tbuf[i - (i && buf[i-1] == '\\r')] = 0;\n+\t\t\t\ttrim_trailing_comments(entry);\n \t\t\t\ttrim_trailing_spaces(entry);\n \t\t\t\tadd_pattern(entry, base, baselen, pl, lineno);\n \t\t\t}\n-- \ngitgitgadget\n"},{"id":"383253","messageId":"xmqqtv8rej30.fsf@gitster-ct.c.googlers.com","threadId":"51952","inReplyTo":"pull.370.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/1] gitignore: ignore comments on the same line as a pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-02T06:51:15Z","receivedAt":"2019-10-02T06:51:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Chris Zehner via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> Why make this change?\n> =====================\n>\n> Add the ability to use # to put comments after ignore patterns. This is\n> useful for documenting the reason for particular ignore patterns inclusion\n> and structure.\n>\n> Right now a common convention in .gitignore files is to group patterns by\n> similarity, using new lines beginning with one or more # characters as\n> headings to explain these groupings. This works well when clarifying why\n> broad classes of things are ignored, e.g. # build artifacts followed by\n> several patterns.\n>\n> When leaving comments about a particular pattern it can be difficult to\n> distinguish comments about a single patterns from comments used for file\n> organization.\n\nNot a good enough justification to break backward compatibility, I\nwould have to say.  If a single line is so tricky to deserve its own\ncomment, perhaps a group of lines can be separated with the next\ngroup by a handful of blank lines for clarity (or the project can\nadopt a convention that differentiates between comments on a group\nand comments on an individual rule).\n"},{"id":"383254","messageId":"xmqqpnjfeijo.fsf@gitster-ct.c.googlers.com","threadId":"51952","inReplyTo":"e448214e87b46dc649781b10727043bc390d9aab.1569985991.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] gitignore: ignore comments on the same line as a pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-02T07:02:51Z","receivedAt":"2019-10-02T07:02:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Chris Zehner via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> -   that begin with a hash.\n> +   Put a backslash (\"`\\`\") in front of each hash for patterns\n> +   containing a hash.\n> +\n> + - A # after a pattern will be treated as the start of a comment.\n> +   Put a backslash (\"`\\`\") in front of each hash for patterns\n> +   containing a hash.\n\nBesides being backward incompatible, this looks somewhat misdesigned\nby lacking a way to escape a backslash (i.e. what should a project\ndo if they want a patttern with backslash followed by a hash\nliterally in there?).\n\n> diff --git a/dir.c b/dir.c\n> index cab9c2a458..aeefe142bc 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -658,6 +658,38 @@ void clear_pattern_list(struct pattern_list *pl)\n>  \tmemset(pl, 0, sizeof(*pl));\n>  }\n>  \n> +static void trim_trailing_comments(char *buf)\n> +{\n> +\tchar *p, *last_hash = NULL;\n> +\tint escape_seq = 0;\n> +\n> +\tfor (p = buf; *p; p++)\n> +\t{\n\nStyle (see Documentation/CodingGuidelines).  The opening parenthesis\nof a control structure like if/for/switch are placed on the same\nline, i.e.\n\n\tfor (p = buf; *p; p++) {\n\nDo we even need a separate 'p', instead of scanning with 'buf' itself?\n\n> +\t\tif (!*p)\n> +\t\t\treturn;\n\nWhat happens when an entry ends with '\\' followed by an EOL?  IOW,\nwhat if escape_seq is true here?\n\n> +\t\tswitch (*p) {\n> +\t\tcase '#':\n> +\t\t\tif (escape_seq)\n> +\t\t\t{\n> +\t\t\t\tescape_seq = 0;\n> +\t\t\t\tp++;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t\tif (!last_hash)\n> +\t\t\t\tlast_hash = p;\n> +\t\t\tbreak;\n> +\t\tcase '\\\\':\n> +\t\t\tescape_seq = 1;\n> +\t\t\tbreak;\n"}]}