GNU bug report logs - #27503
26.0.50; Not lining up Javascript arguments

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 27503 in the body.
You can then email your comments to 27503 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: James Nguyen <jamesn <at> fastmail.com>
To: bug-gnu-emacs <at> gnu.org
Cc: james <at> jojojames.com
Subject: 26.0.50; Not lining up Javascript arguments
Date: Mon, 26 Jun 2017 18:49:59 -0700

I'd like to have js-mode line up arguments normally instead of lining up
arg-wise.

For example:

function functionName(arg1,
                                     arg2) {}

vs

function functionName(arg1,
  arg2)

I think js-mode only support the former at this point. The latter seems
to be fairly common so it'd be great if we could support it.

Following this:
https://emacs.stackexchange.com/questions/29973/stop-javascript-mode-from-lining-up-function-parameters-after-newline/29975#29975

seems to give accurate indentation similar to other editors.

(defun javascript/indent-args (parse-status)
    "Return the proper indentation for the current line."
    (save-excursion
      (back-to-indentation)
      (cond ((nth 4 parse-status)    ; inside comment
             (js--get-c-offset 'c (nth 8 parse-status)))
            ((nth 3 parse-status) 0) ; inside string
            ((eq (char-after) ?#) 0)
            ((save-excursion (js--beginning-of-macro)) 4)
            ;; Indent array comprehension continuation lines specially.
            ((let ((bracket (nth 1 parse-status))
                   beg)
               (and bracket
                    (not (js--same-line bracket))
                    (setq beg (js--indent-in-array-comp bracket))
                    ;; At or after the first loop?
                    (>= (point) beg)
                    (js--array-comp-indentation bracket beg))))
            ((js--chained-expression-p))
            ((js--ctrl-statement-indentation))
            ((js--multi-line-declaration-indentation))
            ((nth 1 parse-status)
             ;; A single closing paren/bracket should be indented at the
             ;; same level as the opening statement. Same goes for
             ;; "case" and "default".
             (let ((same-indent-p (looking-at "[]})]"))
                   (switch-keyword-p (looking-at "default\\_>\\|case\\_>[^:]"))
                   (continued-expr-p (js--continued-expression-p)))
               (goto-char (nth 1 parse-status)) ; go to the opening char
               (progn ; nothing following the opening paren/bracket
                 (skip-syntax-backward " ")
                 (when (eq (char-before) ?\)) (backward-list))
                 (back-to-indentation)
                 (js--maybe-goto-declaration-keyword-end parse-status)
                 (let* ((in-switch-p (unless same-indent-p
                                       (looking-at "\\_<switch\\_>")))
                        (same-indent-p (or same-indent-p
                                           (and switch-keyword-p
                                                in-switch-p)))
                        (indent
                         (cond (same-indent-p
                                (current-column))
                               (continued-expr-p
                                (+ (current-column) (* 2 js-indent-level)
                                   js-expr-indent-offset))
                               (t
                                (+ (current-column) js-indent-level
                                   (pcase (char-after (nth 1 parse-status))
                                     (?\( js-paren-indent-offset)
                                     (?\[ js-square-indent-offset)
                                     (?\{ js-curly-indent-offset)))))))
                   (if in-switch-p
                       (+ indent js-switch-indent-offset)
                     indent)))))

            ((js--continued-expression-p)
             (+ js-indent-level js-expr-indent-offset))
            (t (prog-first-column)))))


  (advice-add 'js--proper-indentation :override 'javascript/indent-args)
  
1. This removes an entire if block check. That is probably doing
something I'm not aware of.

2. We probably want to make it configuration either way (something
similar to js-comment-lineup-func (but that seems to only be for comments.))

Some source code to play with is:

  #+begin_src mhtml :tangle yes
<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8">
    <title>Lifecycle</title>
    <style>
      body, textarea {
        font-family: Courier;
      }
    </style>
  </head>
  <body>
    <div id="app">
      <!-- my app renders here -->
    </div>
    <script src="react/build/react.js"></script>
    <script src="react/build/react-dom.js"></script>
    <script>
      var logMixin = {
        _log: function(methodName, args) {
          console.log(this.name + '::' + methodName, args);
        },
        componentWillUpdate:  function() {this._log('componentWillUpdate',  arguments);},
        componentDidUpdate:   function() {this._log('componentDidUpdate',   arguments);},
        componentWillMount:   function() {this._log('componentWillMount',   arguments);},
        componentDidMount:    function() {this._log('componentDidMount',    arguments);},
        componentWillUnmount: function() {this._log('componentWillUnmount', arguments);},
      };

      var TextAreaCounter = React.createClass({
        name: 'TextAreaCounter',
        mixins: [logMixin],

        propTypes: {
          defaultValue: React.PropTypes.string,
        },

        getInitialState: function() {
          return {
            text: this.props.defaultValue,
          };
        },

        _textChange: function(ev) {
          this.setState({
            text: ev.target.value,
          });
        },

        render: function() {
          return React.DOM.div(null,
            React.DOM.textarea({
              value: this.state.text,
              onChange: this._textChange,
            }),
            React.DOM.h3(null, this.state.text.length)
          );
        }
      });

      var myTextAreaCounter = ReactDOM.render(
        React.createElement(TextAreaCounter, {
          defaultValue: "Bob",
        }),
        document.getElementById("app")
      );
    </script>
  </body>
</html>
  #+end_src
  
More specifically:

  #+begin_src mhtml :tangle yes
        render: function() {
          return React.DOM.div(null,
            React.DOM.textarea({
              value: this.state.text,
              onChange: this._textChange,
            }),
            React.DOM.h3(null, this.state.text.length)
          );
        }
  #+end_src
  
With the current indent settings, we get:

  #+begin_src mhtml :tangle yes
        render: function() {
          return React.DOM.div(null,
                               React.DOM.textarea({
                                 value: this.state.text,
                                 onChange: this._textChange,
                               }),
                               React.DOM.h3(null, this.state.text.length)
                              );
        }
  #+end_src
  
With the above advice:

#+begin_src mhtml :tangle yes
        render: function() {
          return React.DOM.div(null,
            React.DOM.textarea({
              value: this.state.text,
              onChange: this._textChange,
            }),
            React.DOM.h3(null, this.state.text.length)
          );
        }
#+end_src


In GNU Emacs 26.0.50 (build 4, x86_64-apple-darwin16.5.0, NS appkit-1504.82 Version 10.12.4 (Build 16E195))
of 2017-06-24 built on jamesretina.local
Repository revision: 16d2695674a4c8abbec846c427fe8abef97e07ef
Windowing system distributor 'Apple', version 10.3.1504
Recent messages:
The following feature was found in load-path, please check if that’s correct:
(obarray)
Successfully reloaded Org
Org-mode version 8.2.10 (release_8.2.10 @ /Users/james/Code/emacs/nextstep/Emacs.app/Contents/Resources/lisp/org/)
Mark set
Configuring package helm...
Configuring package tramp...done
Configuring package helm...done (0.310s)
Configuring package helm-flx...done
Configuring package helm-fuzzier...done

Configured using:
'configure --with-ns'

Configured features:
JPEG RSVG NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Org

Minor modes in effect:
  helm-fuzzier-mode: t
  helm-flx-mode: t
  helm-mode: t
  helm-autoresize-mode: t
  helm--remap-mouse-mode: t
  shell-dirtrack-mode: t
  focus-autosave-mode: t
  company-quickhelp-mode: t
  company-quickhelp-local-mode: t
  eval-sexp-fu-flash-mode: t
  flycheck-pos-tip-mode: t
  shackle-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  global-company-mode: t
  company-mode: t
  global-evil-surround-mode: t
  evil-surround-mode: t
  global-evil-visualstar-mode: t
  evil-visualstar-mode: t
  global-evil-matchit-mode: t
  evil-matchit-mode: t
  evil-mode: t
  evil-local-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  recentf-mode: t
  ivy-mode: t
  smartparens-global-mode: t
  smartparens-mode: t
  global-hungry-delete-mode: t
  hungry-delete-mode: t
  ws-butler-global-mode: t
  ws-butler-mode: t
  show-paren-mode: t
  global-auto-revert-mode: t
  winner-mode: t
  override-global-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/Users/james/.emacs.d/elpa/26/color-theme-solarized-20160626.743/solarized-theme hides /Users/james/.emacs.d/elpa/26/solarized-theme-20170430.800/solarized-theme
~/.emacs.d/fork/evil/evil hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil
~/.emacs.d/fork/evil/evil-vars hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-vars
~/.emacs.d/fork/evil/evil-types hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-types
~/.emacs.d/fork/evil/evil-states hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-states
~/.emacs.d/fork/evil/evil-search hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-search
~/.emacs.d/fork/evil/evil-repeat hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-repeat
~/.emacs.d/fork/evil/evil-pkg hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-pkg
~/.emacs.d/fork/evil/evil-maps hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-maps
~/.emacs.d/fork/evil/evil-macros hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-macros
~/.emacs.d/fork/evil/evil-jumps hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-jumps
~/.emacs.d/fork/evil/evil-integration hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-integration
~/.emacs.d/fork/evil/evil-ex hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-ex
~/.emacs.d/fork/evil/evil-digraphs hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-digraphs
~/.emacs.d/fork/evil/evil-core hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-core
~/.emacs.d/fork/evil/evil-common hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-common
~/.emacs.d/fork/evil/evil-commands hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-commands
~/.emacs.d/fork/evil/evil-command-window hides /Users/james/.emacs.d/elpa/26/evil-20170615.1320/evil-command-window

Features:
(shadow sort mail-extr emacsbug message rfc822 mml mml-sec epa epg
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail helm-fuzzier helm-flx helm-smex helm-command
helm-elisp helm-eval helm-mode helm-files image-dired tramp tramp-compat
tramp-loaddefs trampver parse-time dired-x dired-aux helm-buffers
helm-tags helm-bookmark helm-adaptive helm-info bookmark pp helm-locate
helm-grep helm-regexp helm-external helm-net helm-utils compile
helm-help helm-types helm helm-source eieio-compat helm-multi-match
helm-lib async smex ido loadhist solarized-light-theme solarized add-log
server pulse shell tabify org-element org-rmail org-mhe org-irc org-info
org-gnus org-docview doc-view image-mode dired dired-loaddefs org-bibtex
bibtex org-bbdb org-w3m org org-macro org-footnote org-pcomplete
pcomplete org-list org-faces org-entities org-version ob-emacs-lisp ob
ob-tangle ob-ref ob-lob ob-table ob-exp org-src ob-keys ob-comint comint
ansi-color ob-core ob-eval org-compat org-macs org-loaddefs JJ-org
cursor-sensor mhtml-mode rainbow-mode xterm-color css-mode smie eww puny
mm-url gnus nnheader gnus-util rmail rmail-loaddefs rfc2047 rfc2045
ietf-drums mail-utils mm-util mail-prsvr url-queue url url-proxy
url-privacy url-expand url-methods url-history url-cookie url-domsuf
url-util mailcap shr svg xml browse-url format-spec js cc-mode cc-fonts
cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs
smartparens-html sgml-mode dom JJ-web focus-autosave-mode JJ-security
colir color counsel jka-compr esh-util rainbow-delimiters
evil-cleverparens evil-cleverparens-text-objects evil-cleverparens-util
paredit lispyville lispy swiper iedit iedit-lib multiple-cursors-core
lispy-inline avy semantic/db eieio-base semantic/util-modes
semantic/util semantic semantic/tag semantic/lex semantic/fw mode-local
cedet ediff-merg ediff-wind ediff-diff ediff-mult ediff-help ediff-init
ediff-util ediff help-fns radix-tree lispy-tags elisp-slime-nav
eval-sexp-fu company-quickhelp warnings highlight font-lock+
flycheck-pos-tip pos-tip flycheck json map find-func shackle
JJ-extra-lang make-mode JJ-elisp edebug-x edebug which-func imenu
JJ-autocomplete elixir-yasnippets yasnippet company-oddmuse
company-keywords company-etags etags xref project company-gtags
company-files company-capf company-cmake company-xcode company-clang
company-semantic company-eclim company-template company-css company-nxml
company-dabbrev-code company-dabbrev company-yasnippet company-bbdb
company JJ-evil evil-surround evil-visualstar evil-matchit evil
evil-integration evil-maps evil-commands flyspell ispell evil-jumps
evil-command-window evil-types evil-search evil-ex evil-macros
evil-repeat evil-states evil-core evil-common derived rect evil-digraphs
evil-vars undo-tree diff JJ-project recentf tree-widget wid-edit ivy flx
delsel ivy-overlay ffap JJ-pair-editing smartparens-config smartparens
thingatpt JJ-misc fold-dwim-org fold-dwim hideshow noutline outline
windmove hungry-delete ws-butler JJ-platform exec-path-from-shell
ls-lisp JJ-defaults paren whitespace autorevert filenotify winner
JJ-theme foggy-night-theme cl-extra help-mode theme-changer solar
cal-dst cal-menu calendar cal-loaddefs cl JJ-dependencies hydra ring lv
s dash JJ-funcs subr-x use-package diminish bind-key easy-mmode
finder-inf edmacro kmacro rx advice slime-autoloads info package
easymenu epg-config url-handlers url-parse auth-source cl-seq eieio
eieio-core cl-macs eieio-loaddefs password-cache url-vars seq byte-opt
gv bytecomp byte-compile cconv cl-loaddefs pcase cl-lib time-date
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/ns-win ns-win ucs-normalize mule-util term/common-win
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow isearch timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core term/tty-colors frame cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote kqueue cocoa ns
multi-tty make-network-process emacs)

Memory information:
((conses 16 892577 534002)
(symbols 48 60548 281)
(miscs 40 882 3402)
(strings 32 161034 279476)
(string-bytes 1 5481778)
(vectors 16 98335)
(vector-slots 8 2434861 513384)
(floats 8 884 2213)
(intervals 56 6906 1373)
(buffers 976 24))

Message #8 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: Ingo Lohmar <i.lohmar <at> gmail.com>
To: James Nguyen <jamesn <at> fastmail.com>, 27503 <at> debbugs.gnu.org
Cc: james <at> jojojames.com
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Tue, 27 Jun 2017 20:16:14 +0200

I've been meaning to post this for months, guess now is as good a time
as any.  Here's an alternative take that I've been using for a long time
without noticing any bugs.  It is a less intrusive change to achieve the
desired effect AFAICT:



diff --git i/lisp/progmodes/js.el w/lisp/progmodes/js.el
index bae9e52bf0..a27db82eb0 100644
--- i/lisp/progmodes/js.el
+++ w/lisp/progmodes/js.el
@@ -475,6 +475,11 @@ js-flat-functions
   :type 'boolean
   :group 'js)
 
+(defcustom js-cont-nonempty-list-indent-rigidly nil
+  "Indent continuation of non-empty ([{ lines in `js-mode' rigidly."
+  :type 'boolean
+  :group 'js)
+
 (defcustom js-comment-lineup-func #'c-lineup-C-comments
   "Lineup function for `cc-mode-style', for C comments in `js-mode'."
   :type 'function
@@ -2092,7 +2097,8 @@ js--proper-indentation
                  (switch-keyword-p (looking-at "default\\_>\\|case\\_>[^:]"))
                  (continued-expr-p (js--continued-expression-p)))
              (goto-char (nth 1 parse-status)) ; go to the opening char
-             (if (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)")
+             (if (or js-cont-nonempty-list-indent-rigidly
+                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)"))
                  (progn ; nothing following the opening paren/bracket
                    (skip-syntax-backward " ")
                    (when (eq (char-before) ?\)) (backward-list))

Message #11 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: James Nguyen <jamesn <at> fastmail.com>
To: Ingo Lohmar <i.lohmar <at> gmail.com>, James Nguyen <jamesn <at> fastmail.com>,
 27503 <at> debbugs.gnu.org
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Tue, 27 Jun 2017 12:42:34 -0700

This would effectively be the same as my snippet right? I removed the if
check and the else block to always go into the progn.

This looks good to me unless there are other gotchas. Making it
configurable is ideal.

-- 
  James Nguyen
  jamesn <at> fastmail.com

On Tue, Jun 27, 2017, at 11:16 AM, Ingo Lohmar wrote:
> I've been meaning to post this for months, guess now is as good a time
> as any.  Here's an alternative take that I've been using for a long time
> without noticing any bugs.  It is a less intrusive change to achieve the
> desired effect AFAICT:
> 
> 
> 
> diff --git i/lisp/progmodes/js.el w/lisp/progmodes/js.el
> index bae9e52bf0..a27db82eb0 100644
> --- i/lisp/progmodes/js.el
> +++ w/lisp/progmodes/js.el
> @@ -475,6 +475,11 @@ js-flat-functions
>    :type 'boolean
>    :group 'js)
>  
> +(defcustom js-cont-nonempty-list-indent-rigidly nil
> +  "Indent continuation of non-empty ([{ lines in `js-mode' rigidly."
> +  :type 'boolean
> +  :group 'js)
> +
>  (defcustom js-comment-lineup-func #'c-lineup-C-comments
>    "Lineup function for `cc-mode-style', for C comments in `js-mode'."
>    :type 'function
> @@ -2092,7 +2097,8 @@ js--proper-indentation
>                   (switch-keyword-p (looking-at
>                   "default\\_>\\|case\\_>[^:]"))
>                   (continued-expr-p (js--continued-expression-p)))
>               (goto-char (nth 1 parse-status)) ; go to the opening char
> -             (if (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)")
> +             (if (or js-cont-nonempty-list-indent-rigidly
> +                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)"))
>                   (progn ; nothing following the opening paren/bracket
>                     (skip-syntax-backward " ")
>                     (when (eq (char-before) ?\)) (backward-list))

Message #14 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: Ingo Lohmar <i.lohmar <at> gmail.com>
To: James Nguyen <jamesn <at> fastmail.com>, James Nguyen <jamesn <at> fastmail.com>,
 27503 <at> debbugs.gnu.org
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Tue, 27 Jun 2017 22:44:50 +0200

On Tue, Jun 27 2017 12:42 (-0700), James Nguyen wrote:

> This would effectively be the same as my snippet right? I removed the if
> check and the else block to always go into the progn.

You're right, and I am sorry for my bad choice of words, I did not mean
to imply otherwise.  In fact I have to admit I did not check your code
in detail, I just noticed that the behavior was fixed (like you stated).

Thanks for starting the bug report, I would be happy if that change
would land in Emacs.

Message #17 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: James Nguyen <jamesn <at> fastmail.com>
To: Ingo Lohmar <i.lohmar <at> gmail.com>
Cc: 27503 <at> debbugs.gnu.org
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Tue, 27 Jun 2017 17:45:30 -0700

No problem, I was just confirming.

I prefer your snippet anyhow.

> On Jun 27, 2017, at 1:44 PM, Ingo Lohmar <i.lohmar <at> gmail.com> wrote:
> 
> On Tue, Jun 27 2017 12:42 (-0700), James Nguyen wrote:
> 
>> This would effectively be the same as my snippet right? I removed the if
>> check and the else block to always go into the progn.
> 
> You're right, and I am sorry for my bad choice of words, I did not mean
> to imply otherwise.  In fact I have to admit I did not check your code
> in detail, I just noticed that the behavior was fixed (like you stated).
> 
> Thanks for starting the bug report, I would be happy if that change
> would land in Emacs.

Message #20 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Ingo Lohmar <i.lohmar <at> gmail.com>, James Nguyen <jamesn <at> fastmail.com>,
 27503 <at> debbugs.gnu.org
Cc: james <at> jojojames.com
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Wed, 28 Jun 2017 13:35:17 +0300

Hey Ingo,

On 6/27/17 9:16 PM, Ingo Lohmar wrote:
> diff --git i/lisp/progmodes/js.el w/lisp/progmodes/js.el
> index bae9e52bf0..a27db82eb0 100644
> --- i/lisp/progmodes/js.el
> +++ w/lisp/progmodes/js.el
> @@ -475,6 +475,11 @@ js-flat-functions
>     :type 'boolean
>     :group 'js)
>   
> +(defcustom js-cont-nonempty-list-indent-rigidly nil
> +  "Indent continuation of non-empty ([{ lines in `js-mode' rigidly."
> +  :type 'boolean
> +  :group 'js)

The code looks okay to me. Any particular reason to call this variable
"-rigidly"? What does that mean?

Message #23 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: James Nguyen <jamesn <at> fastmail.com>, Ingo Lohmar <i.lohmar <at> gmail.com>,
 27503 <at> debbugs.gnu.org
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Wed, 28 Jun 2017 13:36:40 +0300

On 6/27/17 10:42 PM, James Nguyen wrote:
> This would effectively be the same as my snippet right?

You are comparing a wall of "this works for me" code with a proper patch 
submission.

Message #26 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Petton <nicolas <at> petton.fr>
To: James Nguyen <jamesn <at> fastmail.com>, Ingo Lohmar <i.lohmar <at> gmail.com>,
 James Nguyen <jamesn <at> fastmail.com>, 27503 <at> debbugs.gnu.org
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Wed, 28 Jun 2017 12:57:54 +0200

[Message part 1 (text/plain, inline)]

James Nguyen <jamesn <at> fastmail.com> writes:

> This would effectively be the same as my snippet right? I removed the if
> check and the else block to always go into the progn.
>
> This looks good to me unless there are other gotchas. Making it
> configurable is ideal.

The patch looks good to me. I have been using a similar one for quite
some time.

Cheers,
Nico

[signature.asc (application/pgp-signature, inline)]

Message #29 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: Ingo Lohmar <i.lohmar <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 27503 <at> debbugs.gnu.org, James Nguyen <jamesn <at> fastmail.com>,
 james <at> jojojames.com
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Wed, 28 Jun 2017 16:56:32 +0200

[Message part 1 (text/plain, inline)]

Rigidly was just meant as the opposite of the default "aligned"
indentation, I am not attached to the variable name *at all*. Maybe
...-indent-aligned and change the default to t?

On Jun 28, 2017 12:35, "Dmitry Gutov" <dgutov <at> yandex.ru> wrote:

The code looks okay to me. Any particular reason to call this variable
"-rigidly"? What does that mean?

[Message part 2 (text/html, inline)]

Message #32 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Ingo Lohmar <i.lohmar <at> gmail.com>
Cc: 27503 <at> debbugs.gnu.org, James Nguyen <jamesn <at> fastmail.com>,
 james <at> jojojames.com
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Thu, 29 Jun 2017 03:52:31 +0300

On 6/28/17 5:56 PM, Ingo Lohmar wrote:
> Rigidly was just meant as the opposite of the default "aligned" 
> indentation, I am not attached to the variable name *at all*. Maybe 
> ...-indent-aligned and change the default to t?

Sounds good to me. Maybe also add an example file in test/manual/indent?

Message #35 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: Ingo Lohmar <i.lohmar <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 27503 <at> debbugs.gnu.org, James Nguyen <jamesn <at> fastmail.com>,
 james <at> jojojames.com
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Sat, 01 Jul 2017 13:23:20 +0200

On Thu, Jun 29 2017 03:52 (+0300), Dmitry Gutov wrote:

> On 6/28/17 5:56 PM, Ingo Lohmar wrote:
>> Rigidly was just meant as the opposite of the default "aligned" 
>> indentation, I am not attached to the variable name *at all*. Maybe 
>> ...-indent-aligned and change the default to t?
>
> Sounds good to me. Maybe also add an example file in test/manual/indent?

Hi Dmitry,

I hope I understood the manual/indent idea correctly.  Below is the
updated patch including a test file.

I changed the variable name to `js-indent-cont-nonempty-aligned' now
(with "flipped" boolean meaning, as discussed before).  This is more
succinct and starts with `js-indent-...' to convey the feature area to
which this setting belongs.

What's the procedure for patches arising from a bug report --- should I
add a NEWS entry (for 26.1, marked +++) and just commit this to master
myself?



From f0ec15d5fa82b0ca9b4c6aa7032262252ab63e40 Mon Sep 17 00:00:00 2001
From: Ingo Lohmar <i.lohmar <at> gmail.com>
Date: Sat, 1 Jul 2017 13:09:20 +0200
Subject: [PATCH] Offer non-aligned indentation in lists in js-mode (Bug#27503)

* lisp/progmodes/js.el (js--proper-indentation):
New customization option 'js-indent-cont-nonempty-aligned'.  Affects
argument lists as well as arrays and object properties.
* test/manual/indent/js-indent-cont-nonempty-aligned-nil.js: Test the change.
---
 lisp/progmodes/js.el                                 |  8 +++++++-
 .../indent/js-indent-cont-nonempty-aligned-nil.js    | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 test/manual/indent/js-indent-cont-nonempty-aligned-nil.js

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index bae9e52bf0..d284ddae4d 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -475,6 +475,11 @@ js-flat-functions
   :type 'boolean
   :group 'js)
 
+(defcustom js-indent-cont-nonempty-aligned t
+  "Align continuation of non-empty ([{ lines in `js-mode'."
+  :type 'boolean
+  :group 'js)
+
 (defcustom js-comment-lineup-func #'c-lineup-C-comments
   "Lineup function for `cc-mode-style', for C comments in `js-mode'."
   :type 'function
@@ -2092,7 +2097,8 @@ js--proper-indentation
                  (switch-keyword-p (looking-at "default\\_>\\|case\\_>[^:]"))
                  (continued-expr-p (js--continued-expression-p)))
              (goto-char (nth 1 parse-status)) ; go to the opening char
-             (if (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)")
+             (if (or (not js-indent-cont-nonempty-aligned)
+                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)"))
                  (progn ; nothing following the opening paren/bracket
                    (skip-syntax-backward " ")
                    (when (eq (char-before) ?\)) (backward-list))
diff --git a/test/manual/indent/js-indent-cont-nonempty-aligned-nil.js b/test/manual/indent/js-indent-cont-nonempty-aligned-nil.js
new file mode 100644
index 0000000000..428e922fbb
--- /dev/null
+++ b/test/manual/indent/js-indent-cont-nonempty-aligned-nil.js
@@ -0,0 +1,20 @@
+const funcAssignment = function (arg1,
+    arg2,
+    arg3) {
+    return { test: this,
+        which: "would",
+        align: "as well with the default setting"
+    };
+}
+
+function funcDeclaration(arg1,
+    arg2
+) {
+    return [arg1,
+        arg2];
+}
+
+// Local Variables:
+// indent-tabs-mode: nil
+// js-indent-cont-nonempty-aligned: nil
+// End:
-- 
2.11.0

Message #38 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Ingo Lohmar <i.lohmar <at> gmail.com>
Cc: 27503 <at> debbugs.gnu.org, James Nguyen <jamesn <at> fastmail.com>,
 james <at> jojojames.com
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Mon, 3 Jul 2017 05:11:30 +0300

On 7/1/17 2:23 PM, Ingo Lohmar wrote:

> I hope I understood the manual/indent idea correctly.  Below is the
> updated patch including a test file.

Looks good, thanks.

> I changed the variable name to `js-indent-cont-nonempty-aligned' now
> (with "flipped" boolean meaning, as discussed before).  This is more
> succinct and starts with `js-indent-...' to convey the feature area to
> which this setting belongs.

OK, it seems better. Still requires effort to decipher the meaning, 
though. I don't have any better suggestions, so might as well commit 
this name.

> What's the procedure for patches arising from a bug report --- should I
> add a NEWS entry (for 26.1, marked +++) and just commit this to master
> myself?

When a reviewer says "Looks good, please install", or you're feeling 
confident yourself, yes. On that note, LGTM, please install. :)

A NEWS entry for the new variable is a good idea. But "+++" means "all 
necessary documentation updates are complete". "---", meaning "no change 
in the manuals is needed", seems more appropriate.

Message #41 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: Ingo Lohmar <i.lohmar <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 27503 <at> debbugs.gnu.org, James Nguyen <jamesn <at> fastmail.com>,
 james <at> jojojames.com
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Mon, 03 Jul 2017 20:05:20 +0200

On Mon, Jul 03 2017 05:11 (+0300), Dmitry Gutov wrote:
>> I changed the variable name to `js-indent-cont-nonempty-aligned' now
>> (with "flipped" boolean meaning, as discussed before).  This is more
>> succinct and starts with `js-indent-...' to convey the feature area to
>> which this setting belongs.
>
> OK, it seems better. Still requires effort to decipher the meaning, 
> though. I don't have any better suggestions, so might as well commit 
> this name.
Changed to the better 'js-indent-align-list-continuations', I think
that's reasonably clear. :)

> A NEWS entry for the new variable is a good idea. But "+++" means "all 
> necessary documentation updates are complete". "---", meaning "no change 
> in the manuals is needed", seems more appropriate.
You're right, done.

I think that the changed variable name is not contentious (and an
improvement), so I'll just go ahead and commit this.  Thanks for the
feedback!

Message #44 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: Ingo Lohmar <i.lohmar <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 27503 <at> debbugs.gnu.org, James Nguyen <jamesn <at> fastmail.com>,
 james <at> jojojames.com
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Mon, 03 Jul 2017 20:21:35 +0200

tags 27503 fixed
close 27503 26.1
quit

Pushed to master:
http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9ac7dccc51ee834b06cdabf6a5746eb375f984f0.

Message #47 received at 27503 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Ingo Lohmar <i.lohmar <at> gmail.com>
Cc: 27503 <at> debbugs.gnu.org, James Nguyen <jamesn <at> fastmail.com>,
 james <at> jojojames.com
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Mon, 3 Jul 2017 23:17:38 +0300

On 7/3/17 9:05 PM, Ingo Lohmar wrote:

> I think that the changed variable name is not contentious (and an
> improvement), so I'll just go ahead and commit this.  Thanks for the
> feedback!

Very good, thanks!

Message #52 received at 27503-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Ingo Lohmar <i.lohmar <at> gmail.com>
Cc: 27503-done <at> debbugs.gnu.org, James Nguyen <jamesn <at> fastmail.com>,
 james <at> jojojames.com
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Tue, 4 Jul 2017 00:01:07 +0300

Version: 26.1

On 7/3/17 9:21 PM, Ingo Lohmar wrote:
> tags 27503 fixed
> close 27503 26.1
> quit

The capricious bug tracker doesn't want to understand this: the bug was 
still open. Should be closed now (see the first Cc: address).

Message #55 received at 27503-done <at> debbugs.gnu.org (full text, mbox):

From: James Nguyen <james <at> jojojames.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>, Ingo Lohmar <i.lohmar <at> gmail.com>
Cc: , 27503-done <at> debbugs.gnu.org, James Nguyen <jamesn <at> fastmail.com>
Subject: Re: bug#27503: 26.0.50; Not lining up Javascript arguments
Date: Mon, 03 Jul 2017 16:14:32 -0700

Dmitry Gutov <dgutov <at> yandex.ru> writes:

> Version: 26.1
>
> On 7/3/17 9:21 PM, Ingo Lohmar wrote:
>> tags 27503 fixed
>> close 27503 26.1
>> quit
>
> The capricious bug tracker doesn't want to understand this: the bug was 
> still open. Should be closed now (see the first Cc: address).

Thanks Dmity and Ingo for working on this!

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.