unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#27503: 26.0.50; Not lining up Javascript arguments
@ 2017-06-27  1:49 James Nguyen
  2017-06-27 18:16 ` Ingo Lohmar
  0 siblings, 1 reply; 17+ messages in thread
From: James Nguyen @ 2017-06-27  1:49 UTC (permalink / raw)
  To: 27503; +Cc: james

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))





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-06-27  1:49 bug#27503: 26.0.50; Not lining up Javascript arguments James Nguyen
@ 2017-06-27 18:16 ` Ingo Lohmar
  2017-06-27 19:42   ` James Nguyen
  2017-06-28 10:35   ` Dmitry Gutov
  0 siblings, 2 replies; 17+ messages in thread
From: Ingo Lohmar @ 2017-06-27 18:16 UTC (permalink / raw)
  To: James Nguyen, 27503; +Cc: james

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))





^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-06-27 18:16 ` Ingo Lohmar
@ 2017-06-27 19:42   ` James Nguyen
  2017-06-27 20:44     ` Ingo Lohmar
                       ` (2 more replies)
  2017-06-28 10:35   ` Dmitry Gutov
  1 sibling, 3 replies; 17+ messages in thread
From: James Nguyen @ 2017-06-27 19:42 UTC (permalink / raw)
  To: Ingo Lohmar, James Nguyen, 27503

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@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))





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-06-27 19:42   ` James Nguyen
@ 2017-06-27 20:44     ` Ingo Lohmar
  2017-06-28  0:45       ` James Nguyen
  2017-06-28 10:36     ` Dmitry Gutov
  2017-06-28 10:57     ` Nicolas Petton
  2 siblings, 1 reply; 17+ messages in thread
From: Ingo Lohmar @ 2017-06-27 20:44 UTC (permalink / raw)
  To: James Nguyen, James Nguyen, 27503

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.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-06-27 20:44     ` Ingo Lohmar
@ 2017-06-28  0:45       ` James Nguyen
  0 siblings, 0 replies; 17+ messages in thread
From: James Nguyen @ 2017-06-28  0:45 UTC (permalink / raw)
  To: Ingo Lohmar; +Cc: 27503

No problem, I was just confirming.

I prefer your snippet anyhow.

> On Jun 27, 2017, at 1:44 PM, Ingo Lohmar <i.lohmar@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.






^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-06-27 18:16 ` Ingo Lohmar
  2017-06-27 19:42   ` James Nguyen
@ 2017-06-28 10:35   ` Dmitry Gutov
  2017-06-28 14:56     ` Ingo Lohmar
  1 sibling, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2017-06-28 10:35 UTC (permalink / raw)
  To: Ingo Lohmar, James Nguyen, 27503; +Cc: james

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?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-06-27 19:42   ` James Nguyen
  2017-06-27 20:44     ` Ingo Lohmar
@ 2017-06-28 10:36     ` Dmitry Gutov
  2017-06-28 10:57     ` Nicolas Petton
  2 siblings, 0 replies; 17+ messages in thread
From: Dmitry Gutov @ 2017-06-28 10:36 UTC (permalink / raw)
  To: James Nguyen, Ingo Lohmar, 27503

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.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-06-27 19:42   ` James Nguyen
  2017-06-27 20:44     ` Ingo Lohmar
  2017-06-28 10:36     ` Dmitry Gutov
@ 2017-06-28 10:57     ` Nicolas Petton
  2 siblings, 0 replies; 17+ messages in thread
From: Nicolas Petton @ 2017-06-28 10:57 UTC (permalink / raw)
  To: James Nguyen, Ingo Lohmar, James Nguyen, 27503

[-- Attachment #1: Type: text/plain, Size: 364 bytes --]

James Nguyen <jamesn@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-06-28 10:35   ` Dmitry Gutov
@ 2017-06-28 14:56     ` Ingo Lohmar
  2017-06-29  0:52       ` Dmitry Gutov
  0 siblings, 1 reply; 17+ messages in thread
From: Ingo Lohmar @ 2017-06-28 14:56 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 27503, James Nguyen, james

[-- Attachment #1: Type: text/plain, Size: 350 bytes --]

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@yandex.ru> wrote:

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

[-- Attachment #2: Type: text/html, Size: 650 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-06-28 14:56     ` Ingo Lohmar
@ 2017-06-29  0:52       ` Dmitry Gutov
  2017-07-01 11:23         ` Ingo Lohmar
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2017-06-29  0:52 UTC (permalink / raw)
  To: Ingo Lohmar; +Cc: 27503, James Nguyen, james

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?





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-06-29  0:52       ` Dmitry Gutov
@ 2017-07-01 11:23         ` Ingo Lohmar
  2017-07-03  2:11           ` Dmitry Gutov
  0 siblings, 1 reply; 17+ messages in thread
From: Ingo Lohmar @ 2017-07-01 11:23 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 27503, James Nguyen, james

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@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






^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-07-01 11:23         ` Ingo Lohmar
@ 2017-07-03  2:11           ` Dmitry Gutov
  2017-07-03 18:05             ` Ingo Lohmar
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2017-07-03  2:11 UTC (permalink / raw)
  To: Ingo Lohmar; +Cc: 27503, James Nguyen, james

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.





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-07-03  2:11           ` Dmitry Gutov
@ 2017-07-03 18:05             ` Ingo Lohmar
  2017-07-03 18:21               ` Ingo Lohmar
  2017-07-03 20:17               ` Dmitry Gutov
  0 siblings, 2 replies; 17+ messages in thread
From: Ingo Lohmar @ 2017-07-03 18:05 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 27503, James Nguyen, james

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!





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-07-03 18:05             ` Ingo Lohmar
@ 2017-07-03 18:21               ` Ingo Lohmar
  2017-07-03 21:01                 ` Dmitry Gutov
  2017-07-03 20:17               ` Dmitry Gutov
  1 sibling, 1 reply; 17+ messages in thread
From: Ingo Lohmar @ 2017-07-03 18:21 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 27503, James Nguyen, james

tags 27503 fixed
close 27503 26.1
quit

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





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-07-03 18:05             ` Ingo Lohmar
  2017-07-03 18:21               ` Ingo Lohmar
@ 2017-07-03 20:17               ` Dmitry Gutov
  1 sibling, 0 replies; 17+ messages in thread
From: Dmitry Gutov @ 2017-07-03 20:17 UTC (permalink / raw)
  To: Ingo Lohmar; +Cc: 27503, James Nguyen, james

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!





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-07-03 18:21               ` Ingo Lohmar
@ 2017-07-03 21:01                 ` Dmitry Gutov
  2017-07-03 23:14                   ` James Nguyen
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Gutov @ 2017-07-03 21:01 UTC (permalink / raw)
  To: Ingo Lohmar; +Cc: 27503-done, James Nguyen, james

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).





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#27503: 26.0.50; Not lining up Javascript arguments
  2017-07-03 21:01                 ` Dmitry Gutov
@ 2017-07-03 23:14                   ` James Nguyen
  0 siblings, 0 replies; 17+ messages in thread
From: James Nguyen @ 2017-07-03 23:14 UTC (permalink / raw)
  To: Dmitry Gutov, Ingo Lohmar; +Cc: 27503-done, James Nguyen

Dmitry Gutov <dgutov@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!





^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2017-07-03 23:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-27  1:49 bug#27503: 26.0.50; Not lining up Javascript arguments James Nguyen
2017-06-27 18:16 ` Ingo Lohmar
2017-06-27 19:42   ` James Nguyen
2017-06-27 20:44     ` Ingo Lohmar
2017-06-28  0:45       ` James Nguyen
2017-06-28 10:36     ` Dmitry Gutov
2017-06-28 10:57     ` Nicolas Petton
2017-06-28 10:35   ` Dmitry Gutov
2017-06-28 14:56     ` Ingo Lohmar
2017-06-29  0:52       ` Dmitry Gutov
2017-07-01 11:23         ` Ingo Lohmar
2017-07-03  2:11           ` Dmitry Gutov
2017-07-03 18:05             ` Ingo Lohmar
2017-07-03 18:21               ` Ingo Lohmar
2017-07-03 21:01                 ` Dmitry Gutov
2017-07-03 23:14                   ` James Nguyen
2017-07-03 20:17               ` Dmitry Gutov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).