New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gitignore: ignore comments on the same line as a pattern #370
gitignore: ignore comments on the same line as a pattern #370
Conversation
Signed-off-by: Chris Zehner <cbzehner@gmail.com>
Welcome to GitGitGadgetHi @cbzehner, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests. Please make sure that this Pull Request has a good description, as it will be used as cover letter. Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:
It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code. Contributing the patchesBefore you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a PR comment of the form Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions. If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox ("raw") file corresponding to the mail you want to reply to from the Git mailing list. If you use GMail, you can upload that raw mbox file via: curl -g --user "<EMailAddress>:<Password>" --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt |
/allow |
User cbzehner is now allowed to use GitGitGadget. WARNING: cbzehner has no public email address set on GitHub |
/submit |
Submitted as pull.370.git.gitgitgadget@gmail.com |
On the Git mailing list, Junio C Hamano wrote (reply to this):
|
@@ -74,8 +74,12 @@ PATTERN FORMAT | |||
for readability. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Chris Zehner via GitGitGadget" <gitgitgadget@gmail.com> writes:
> - that begin with a hash.
> + Put a backslash ("`\`") in front of each hash for patterns
> + containing a hash.
> +
> + - A # after a pattern will be treated as the start of a comment.
> + Put a backslash ("`\`") in front of each hash for patterns
> + containing a hash.
Besides being backward incompatible, this looks somewhat misdesigned
by lacking a way to escape a backslash (i.e. what should a project
do if they want a patttern with backslash followed by a hash
literally in there?).
> diff --git a/dir.c b/dir.c
> index cab9c2a458..aeefe142bc 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -658,6 +658,38 @@ void clear_pattern_list(struct pattern_list *pl)
> memset(pl, 0, sizeof(*pl));
> }
>
> +static void trim_trailing_comments(char *buf)
> +{
> + char *p, *last_hash = NULL;
> + int escape_seq = 0;
> +
> + for (p = buf; *p; p++)
> + {
Style (see Documentation/CodingGuidelines). The opening parenthesis
of a control structure like if/for/switch are placed on the same
line, i.e.
for (p = buf; *p; p++) {
Do we even need a separate 'p', instead of scanning with 'buf' itself?
> + if (!*p)
> + return;
What happens when an entry ends with '\' followed by an EOL? IOW,
what if escape_seq is true here?
> + switch (*p) {
> + case '#':
> + if (escape_seq)
> + {
> + escape_seq = 0;
> + p++;
> + break;
> + }
> + if (!last_hash)
> + last_hash = p;
> + break;
> + case '\\':
> + escape_seq = 1;
> + break;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cbzehner did you see this reply?
Why make this change?
Add the ability to use
#
to put comments after ignore patterns. This isuseful for documenting the reason for particular ignore patterns inclusion
and structure.
Right now a common convention in
.gitignore
files is to group patternsby similarity, using new lines beginning with one or more
#
charactersas headings to explain these groupings. This works well when clarifying
why broad classes of things are ignored, e.g.
# build artifacts
followedby several patterns.
When leaving comments about a particular pattern it can be difficult to
distinguish comments about a single patterns from comments used for file
organization.
Comments left after a pattern are unambiguously related to that line, and
that line only.
What should this change do?
The entirety of a string after a non-escaped
#
, including the#
,is removed from the pattern in a
.gitignore
file.Why make the change this way?
I don't normally write C, so I probably overlooked more idiomatic ways
to do this. This is done similarly to the way
trim_trailing_spaces
removesextraneous spaces from patterns.
Potentially this change could be combined with the existing code for
trim_trailing_spaces
, but the logic is slightly different and it seems to methat the difference in naming aids readability by making it clear what the
responsibilities are for each function.
How can we test this change works?
That's one area I'd like help with, please.
Test cases:
I wasn't sure where the correct place to add these would be, I didn't see (and
potentially overlooked) any tests in
/t/*
that cover this functionality. Wouldsomeone be willing to provide a pointer to the correct place to add these tests?
Signed-off-by: Chris Zehner cbzehner@gmail.com