[Top][All Lists]

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

Re: [PATCH] Update js-mode function heading regexes

From: Dmitry Gutov
Subject: Re: [PATCH] Update js-mode function heading regexes
Date: Fri, 29 Sep 2017 00:02:52 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Thunderbird/56.0

Hi Adam,

Sorry for the late reply.

On 6/19/17 10:04 AM, Adam Niederer wrote:

The file I included was primarily for visual evaluation of the changes.
Adding some automated tests is a good idea, though, so I've included an
updated patchset below. This also makes js--function-heading-2-re work
on async functions.

Sometimes it's easier to evaluate the improvement just by looking at the tests, though it depends on how they are written, of course.

To comment on the patch and your original post:

- The change to heading-3-re is not mentioned, but it's fairly obvious.

- I don't quite understand the description of the change in heading-2-re. Why not at least require a colon?

Not much. Patch submissions via the bug tracker have higher odds of
not being forgotten is they're not applied right away, though.

Thanks for the patch.

Would reposting these changes to bug-gnu-emacs be a good idea?

By now it's looking even better than it did back then. As you can see, it's been lost in the mailing list.

And I'd like for someone else to look at the patch, because it touches the area of js-mode than I've never had to deal with.

+(ert-deftest js-mode-highlight-function-names ()
+  "Ensure js--function-heading-1-re and js--function-heading-2-re match all
+possible function declarations and highlight the function names


+(ert-deftest js-mode-accept-function-bindings ()
+  "Ensure js--function-heading-3-re matches all function declarations
of the
+form (var|let|const) foo = (async)? function."

Regarding this test, can we check something higher level than whether js--function-heading-3-re matches? It's an implementation detail, after all.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]