| Message ID | pull.865.git.git.1601986430388.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | userdiff: support Rust macros | expand |
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 >
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 --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]*)?"