Re: [PATCH] userdiff: add support for Emacs Lisp
flat view
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-02-14 18:26:25
On Sun, Feb 14 2021, Johannes Sixt wrote:
Am 14.02.21 um 02:41 schrieb Ævar Arnfjörð Bjarmason:quoted
Just a cursory "git log -p -- lisp" in emacs.git with your patch shows e.g. lisp/thingatpt.el where forms in a "defun" aren't indented (before it selects the "defun", after with yours it's a "put" in the middle of the function).Note that negative matches can be specified. We use the feature in the cpp case to exclude public:/protected:/private: and label: that happen to be not indented. Perhaps that can be useful here? Oh, and BTW, what the patterns treat as "function" must not match what the language treats as function. The purpose of the hunk header is to spot a place in the source file easily. So, it does not hurt if eval-and-compile forms are captured (as was mentioned in the linked thread) if desired.
Right, so having lots of test-case is helpful, e.g. for elisp maybe you have a top-level defun, maybe not, maybe the top-level is a "(progn" so you'd like a second-level more meaningful context, or maybe not... Obviously these userdiff patterns aren't a general parser and will always be hit-and-miss, it's just useful to at least eyeball them against in-the-wild test data to check their sanity & add some tests. My cursory glance at the emacs.git version v.s. what's being submitted here is that this one does a worse job in *very common* cases.
quoted
Yours also changes it from e.g.: @@ -61,7 +61,7 @@ forward-thing to: @@ -61,7 +61,7 @@ (defun forward-thing (thing &optional n) Is this really desired in elisp?It's common practice to extract the entire line (sans indentation if applicable), not just the function name. Why would somebody want the latter? It doesn't carry as much information as could be possible.
Because I'm familiar with the codebase I'm editing and I just need to
know that the diff I'm viewing is on the "main" function, not that it's
"main()", "int main(int argv, char **argv)", "int main(const int argv,
const char *argv[])", or to either have a " {" or not at the end
depending on the project's coding style.
I know our own userdiff builtin patterns don't do this, but it would be
very useful to retrofit this capability / maybe make it a configurable
feature, i.e. have them capture the meaningful part of the line, and you
could either print it all, or just that part.
quoted
I also note how our tests in t4018-diff-funcname.sh are really bad in not testing for this at all. I.e. we just test that we match the right line, not how we extract a match from it.Oh, well. These are "semi-automated" tests cases. If you have an idea how to achieve the goal without burdening test writers with lots of subtleties, be my guest. ;)
I'll do that soon.
-- Hannes