emacs-devel
[Top][All Lists]
Advanced

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

Re: Requesting patch review


From: Jackson Hamilton
Subject: Re: Requesting patch review
Date: Sun, 8 Mar 2015 15:01:57 -0700

For the sake of continuity I'll send my next patch through this email thread, but in the future I will send them to the bug tracker.

Refactored the code to be less tricky, added the additional level of configuration and ported the tests.

(Note that I had to `cd test/indent` and use `make js.js.test EMACS='../../src/emacs -Q'` to get the tests to run in a clean and correct environment. We may want to update the Makefile to do that by default.

On Sun, Mar 8, 2015 at 10:55 AM, Dmitry Gutov <address@hidden> wrote:
On 03/08/2015 04:09 AM, Jackson Hamilton wrote:

New patch for js-mode. Adds a new indentation option. I would appreciate
review before merging.

Thanks for the patch.

I can see two directions for it to be improved:

- Port the tests. I suppose test/indent/*.js would be a good place for the examples. You can create js-???.js, set the new option's, value using file local variables (in a comment, at the top or the bottom of the file), and then the right indentation would be tested automatically. Run a single file with 'make js.js.test'.

- Add the option for the user to always have this indentation, no matter if there's a comma after the first item or not. js2-mode can do that (here's the original feature request: https://github.com/mooz/js2-mode/issues/3). I suppose it'd be best if the new variable had a different name and 3 possible values (nil - default, t - always, dynamic - check to see if there are several declarations).

By the way, in general it's better to send patches to the bug tracker. They can get lost in emacs-devel if nobody pays attention right away.

Attachment: 0001-New-indentation-option-for-js-mode.patch
Description: Text Data


reply via email to

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