diff mbox series

userdiff: support Rust macros

Message ID pull.865.git.git.1601986430388.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series userdiff: support Rust macros | expand

Commit Message

Elijah Newren via GitGitGadget Oct. 6, 2020, 12:13 p.m. UTC
From: Konrad Borowski <konrad@borowski.pw>

This adds a support for macro_rules! keyword which declares
a macro. It also includes a test case.

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-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-865/xfix/userdiff-macro-rules-rust-v1
Pull-Request: https://github.com/git/git/pull/865

 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

Phillip Wood Oct. 6, 2020, 4:44 p.m. UTC | #1
Hi Konrad

Thanks for the patch, this would be a useful addition

On 06/10/2020 13:13, Konrad Borowski via GitGitGadget wrote:
> From: Konrad Borowski <konrad@borowski.pw>
> 
> This adds a support for macro_rules! keyword which declares
> a macro. It also includes a test case.
> 
> 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-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-865/xfix/userdiff-macro-rules-rust-v1
> Pull-Request: https://github.com/git/git/pull/865
> 
>   t/t4018/rust-macro-rules | 6 ++++++
>   userdiff.c               | 2 +-
>   2 files changed, 7 insertions(+), 1 deletion(-)
>   create mode 100644 t/t4018/rust-macro-rules
> 
> 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..f13a913697 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)[< \t]+|macro_rules[\t ]*!)

I think you want the `!` before the `[\t ]`. We could probably get away 
with just adding `macro-rules!` as another alternative in the group 
starting with `struct`

Best Wishes

Phillip

[^;]*)$",
>   	 /* -- */
>   	 "[a-zA-Z_][a-zA-Z0-9_]*"
>   	 "|[0-9][0-9_a-fA-Fiosuxz]*(\\.([0-9]*[eE][+-]?)?[0-9_fF]*)?"
> 
> base-commit: d98273ba77e1ab9ec755576bc86c716a97bf59d7
>
Johannes Sixt Oct. 6, 2020, 6:44 p.m. UTC | #2
Am 06.10.20 um 14:13 schrieb Konrad Borowski via GitGitGadget:
> From: Konrad Borowski <konrad@borowski.pw>
> 
> This adds a support for macro_rules! keyword which declares
> a macro. It also includes a test case.

We would write this sentence in imperative mood, i.e., give the codebase
the order to become so. That a test case is included can be seen from
the patch text and need not be mentioned.

In this case, if I were the only one to judge, I would accept this
commit without a message if the commit summary were:

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

> 
> 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-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-865/xfix/userdiff-macro-rules-rust-v1
> Pull-Request: https://github.com/git/git/pull/865
> 
>  t/t4018/rust-macro-rules | 6 ++++++
>  userdiff.c               | 2 +-
>  2 files changed, 7 insertions(+), 1 deletion(-)
>  create mode 100644 t/t4018/rust-macro-rules
> 
> 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..f13a913697 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)[< \t]+|macro_rules[\t ]*!)[^;]*)$",
>  	 /* -- */
>  	 "[a-zA-Z_][a-zA-Z0-9_]*"
>  	 "|[0-9][0-9_a-fA-Fiosuxz]*(\\.([0-9]*[eE][+-]?)?[0-9_fF]*)?"

Technically, this looks fine. Originally I thought that the new keyword
could just be thrown into the mix of struct|enum etc, but judging from
the (existing) test cases, this new case looks fairly different (I am
not a Rust expert, so I cannot really tell). Therefore, it is OK to put
it in an alternative branch in the regexp.

Reviewed-by: Johannes Sixt <j6t@kdbg.org>

-- Hannes
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..f13a913697 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)[< \t]+|macro_rules[\t ]*!)[^;]*)$",
 	 /* -- */
 	 "[a-zA-Z_][a-zA-Z0-9_]*"
 	 "|[0-9][0-9_a-fA-Fiosuxz]*(\\.([0-9]*[eE][+-]?)?[0-9_fF]*)?"