diff mbox series

[v2] userdiff: permit Rust's macro_rules! keyword in hunk headers

Message ID pull.865.v2.git.git.1602048429067.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v2] userdiff: permit Rust's macro_rules! keyword in hunk headers | expand

Commit Message

Derrick Stolee via GitGitGadget Oct. 7, 2020, 5:27 a.m. UTC
From: Konrad Borowski <konrad@borowski.pw>

Changes since v1:
- Changed macro_rules! to be considered to use the same
  rule as rest of keywords to reduce the size of a change
  as suggested by Phillip Wood. This means that
  'pub macro_rules!` (a syntax error) is considered to be
  a hunk header.

- Written commit message in imperative mood as suggested by
  Johannes Sixt.

Signed-off-by: Konrad Borowski <konrad@borowski.pw>
---
    userdiff: support Rust macros

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-865%2Fxfix%2Fuserdiff-macro-rules-rust-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-865/xfix/userdiff-macro-rules-rust-v2
Pull-Request: https://github.com/git/git/pull/865

Range-diff vs v1:

 1:  9e071ddf63 ! 1:  a3fd67d84f userdiff: support Rust macros
     @@ Metadata
      Author: Konrad Borowski <konrad@borowski.pw>
      
       ## Commit message ##
     -    userdiff: support Rust macros
     +    userdiff: permit Rust's macro_rules! keyword in hunk headers
      
     -    This adds a support for macro_rules! keyword which declares
     -    a macro. It also includes a test case.
     +    Changes since v1:
     +    - Changed macro_rules! to be considered to use the same
     +      rule as rest of keywords to reduce the size of a change
     +      as suggested by Phillip Wood. This means that
     +      'pub macro_rules!` (a syntax error) is considered to be
     +      a hunk header.
     +
     +    - Written commit message in imperative mood as suggested by
     +      Johannes Sixt.
      
          Signed-off-by: Konrad Borowski <konrad@borowski.pw>
      
     @@ userdiff.c: PATTERNS("ruby", "^[ \t]*((class|module|def)[ \t].*)$",
       	 "|//=?|[-+*/<>%&^|=!]=|<<=?|>>=?|===|\\.{1,3}|::|[!=]~"),
       PATTERNS("rust",
      -	 "^[\t ]*((pub(\\([^\\)]+\\))?[\t ]+)?((async|const|unsafe|extern([\t ]+\"[^\"]+\"))[\t ]+)?(struct|enum|union|mod|trait|fn|impl)[< \t]+[^;]*)$",
     -+	 "^[\t ]*(((pub(\\([^\\)]+\\))?[\t ]+)?((async|const|unsafe|extern([\t ]+\"[^\"]+\"))[\t ]+)?(struct|enum|union|mod|trait|fn|impl)[< \t]+|macro_rules[\t ]*!)[^;]*)$",
     ++	 "^[\t ]*((pub(\\([^\\)]+\\))?[\t ]+)?((async|const|unsafe|extern([\t ]+\"[^\"]+\"))[\t ]+)?(struct|enum|union|mod|trait|fn|impl|macro_rules[\t ]*!)[< \t]+[^;]*)$",
       	 /* -- */
       	 "[a-zA-Z_][a-zA-Z0-9_]*"
       	 "|[0-9][0-9_a-fA-Fiosuxz]*(\\.([0-9]*[eE][+-]?)?[0-9_fF]*)?"


 t/t4018/rust-macro-rules | 6 ++++++
 userdiff.c               | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/rust-macro-rules


base-commit: d98273ba77e1ab9ec755576bc86c716a97bf59d7

Comments

Junio C Hamano Oct. 7, 2020, 6:10 a.m. UTC | #1
"Konrad Borowski via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Konrad Borowski <konrad@borowski.pw>
>
> Changes since v1:
> - Changed macro_rules! to be considered to use the same
>   rule as rest of keywords to reduce the size of a change
>   as suggested by Phillip Wood. This means that
>   'pub macro_rules!` (a syntax error) is considered to be
>   a hunk header.
>
> - Written commit message in imperative mood as suggested by
>   Johannes Sixt.

The above is where you write the proposed log message, not where you
write the differences between v1 and v2 (which, if exists, would be
below the three-dash line after your sign off).  After all, people
who are reading the final history of the project in "git log" will
not even know or care that what they are reading is the second
iteration and/or there was a first iteration that was different from
the final version.

> Subject: [PATCH v2] userdiff: permit Rust's macro_rules! keyword in hunk headers

Somehow I find the "permit" a bit odd---it is not like we are
disallowing Rust programs written in certain way.

     userdiff: recognize 'macro_rules!' as starting a Rust function block

or something?

> Signed-off-by: Konrad Borowski <konrad@borowski.pw>
> ---
> ...

As to the simplified rule that matches 'pub macro_rules!'.

We assume what is written by the end-user, in which these patterns
are used to find function headers, is a syntactically correct
program.  This allows us to write a pattern that may match with a
syntactic nonsense (i.e. false positive is perfectly fine).  What
we care about is to avoid false negatives---if our pattern does not
recognize what users would consider a valid and common function
header line, that is a problem.

So, from that point of view, you do not have to worry too much about
'pub macro_rules!'  matching your pattern.

> diff --git a/t/t4018/rust-macro-rules b/t/t4018/rust-macro-rules
> new file mode 100644
> index 0000000000..ec610c5b62
> --- /dev/null
> +++ b/t/t4018/rust-macro-rules
> @@ -0,0 +1,6 @@
> +macro_rules! RIGHT {
> +    () => {
> +        // a comment
> +        let x = ChangeMe;
> +    };
> +}
> diff --git a/userdiff.c b/userdiff.c
> index fde02f225b..c0f63c57b2 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -165,7 +165,7 @@ PATTERNS("ruby", "^[ \t]*((class|module|def)[ \t].*)$",
>  	 "|[-+0-9.e]+|0[xXbB]?[0-9a-fA-F]+|\\?(\\\\C-)?(\\\\M-)?."
>  	 "|//=?|[-+*/<>%&^|=!]=|<<=?|>>=?|===|\\.{1,3}|::|[!=]~"),
>  PATTERNS("rust",
> -	 "^[\t ]*((pub(\\([^\\)]+\\))?[\t ]+)?((async|const|unsafe|extern([\t ]+\"[^\"]+\"))[\t ]+)?(struct|enum|union|mod|trait|fn|impl)[< \t]+[^;]*)$",
> +	 "^[\t ]*((pub(\\([^\\)]+\\))?[\t ]+)?((async|const|unsafe|extern([\t ]+\"[^\"]+\"))[\t ]+)?(struct|enum|union|mod|trait|fn|impl|macro_rules[\t ]*!)[< \t]+[^;]*)$",

I notice that you still allow "macro_rules    !" as Phillip pointed
out.  Is that intended?

Thanks.
diff mbox series

Patch

diff --git a/t/t4018/rust-macro-rules b/t/t4018/rust-macro-rules
new file mode 100644
index 0000000000..ec610c5b62
--- /dev/null
+++ b/t/t4018/rust-macro-rules
@@ -0,0 +1,6 @@ 
+macro_rules! RIGHT {
+    () => {
+        // a comment
+        let x = ChangeMe;
+    };
+}
diff --git a/userdiff.c b/userdiff.c
index fde02f225b..c0f63c57b2 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -165,7 +165,7 @@  PATTERNS("ruby", "^[ \t]*((class|module|def)[ \t].*)$",
 	 "|[-+0-9.e]+|0[xXbB]?[0-9a-fA-F]+|\\?(\\\\C-)?(\\\\M-)?."
 	 "|//=?|[-+*/<>%&^|=!]=|<<=?|>>=?|===|\\.{1,3}|::|[!=]~"),
 PATTERNS("rust",
-	 "^[\t ]*((pub(\\([^\\)]+\\))?[\t ]+)?((async|const|unsafe|extern([\t ]+\"[^\"]+\"))[\t ]+)?(struct|enum|union|mod|trait|fn|impl)[< \t]+[^;]*)$",
+	 "^[\t ]*((pub(\\([^\\)]+\\))?[\t ]+)?((async|const|unsafe|extern([\t ]+\"[^\"]+\"))[\t ]+)?(struct|enum|union|mod|trait|fn|impl|macro_rules[\t ]*!)[< \t]+[^;]*)$",
 	 /* -- */
 	 "[a-zA-Z_][a-zA-Z0-9_]*"
 	 "|[0-9][0-9_a-fA-Fiosuxz]*(\\.([0-9]*[eE][+-]?)?[0-9_fF]*)?"