Re: [PATCH V4 2/2] object name: introduce '^{/!-<negative pattern>}' notation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Stephen P. Smith" <ischis2@xxxxxxx> writes:

> From: Will Palmer <wmpalmer@xxxxxxxxx>
>
> To name a commit, you can now use the :/!-<negative pattern> regex
> style, and consequentially, say
>
>     $ git rev-parse HEAD^{/!-foo}
>
> and it will return the hash of the first commit reachable from HEAD,
> whose commit message does not contain "foo". This is the opposite of the
> existing <rev>^{/<pattern>} syntax.
>
> The specific use-case this is intended for is to perform an operation,
> excluding the most-recent commits containing a particular marker. For
> example, if you tend to make "work in progress" commits, with messages
> beginning with "WIP", you work, then it could be useful to diff against
> "the most recent commit which was not a WIP commit". That sort of thing
> now possible, via commands such as:
>
>     $ git diff @^{/!-^WIP}
>
> The leader '/!-', rather than simply '/!', to denote a negative match,
> is chosen to leave room for additional modifiers in the future.
>
> Signed-off-by: Will Palmer <wmpalmer@xxxxxxxxx>
> Signed-off-by: Stephen P. Smith <ischis2@xxxxxxx>
> ---
>
> Notes:
>     Changed |say|use the :/!-<negative pattern> regex style, and consequentially, say|.
>     
>     Chose not to chagne subject since it matches the end of
>     git rev-parse HEAD^{/!-foo}
>
>     Mailing list web interface is again not working; therefore, I don't 
>     have URLs for the earlier review comments.

Thanks, this looks good (and it looked good already at the previous
round).

> diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
> index d85e303..0c84d4f 100644
> --- a/Documentation/revisions.txt
> +++ b/Documentation/revisions.txt
> @@ -176,11 +176,12 @@ existing tag object.
>    A colon, followed by a slash, followed by a text, names
>    a commit whose commit message matches the specified regular expression.
>    This name returns the youngest matching commit which is
> -  reachable from any ref.  If the commit message starts with a
> -  '!' you have to repeat that;  the special sequence ':/!',
> -  followed by something else than '!', is reserved for now.
> -  The regular expression can match any part of the commit message. To
> -  match messages starting with a string, one can use e.g. ':/^foo'.
> +  reachable from any ref. The regular expression can match any part of the
> +  commit message. To match messages starting with a string, one can use
> +  e.g. ':/^foo'. The special sequence ':/!' is reserved for modifiers to what
> +  is matched. ':/!-foo' performs a negative match, while ':/!!foo' matches a
> +  literal '!' character, followed by 'foo'. Any other sequence beginning with
> +  ':/!' is reserved for now.

The original text reads as if ":/foo" looks for 'foo' anywhere in
the log message while ":/!!foo" looks for '!foo' at the beginning,
which was incorrect as far as I can tell, but the updated text
corrects it.  Good.

> @@ -903,7 +913,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1,
>  			continue;
>  		buf = get_commit_buffer(commit, NULL);
>  		p = strstr(buf, "\n\n");
> -		matches = p && !regexec(&regex, p + 2, 0, NULL, 0);
> +		matches = p && (negative ^ !regexec(&regex, p + 2, 0, NULL, 0));
>  		unuse_commit_buffer(commit, buf);

Hmph, without "negative pattern match", if you asked for ":/foo" and
the commit did not have any body (which I do not think the current
version of Git allows to create by default, but there may be such
commits created by older versions of Git or reimplementation of Git
made by others), p could be NULL.  In such a case, any regex would
not match, so I would expect that commit to be shown.

In other words, I wonder if the above should be

		matches = negative ^ (p && !regexec(&regex, p + 2, 0, NULL, 0));

This would not make practical difference, but I would expect any
change to introduce "negative patch" to an original logic that is

	matches = ORIGINAL_LOGIC_TO_COMPUTE_MATCH

to become

	matches = negative ^ ORIGINAL_LOGIC_TO_COMPUTE_MATCH


--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]