From: Dmitry Gutov <dgutov@yandex.ru>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 16593@debbugs.gnu.org, Bozhidar Batsov <bozhidar@batsov.com>
Subject: bug#16593: 24.3.50; ruby-mode: align chained method calls on multiple lines
Date: Fri, 31 Jan 2014 05:42:14 +0200 [thread overview]
Message-ID: <52EB1B96.4020602@yandex.ru> (raw)
In-Reply-To: <jwvmwid5ypp.fsf-monnier+emacsbugs@gnu.org>
[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]
On 30.01.2014 16:35, Stefan Monnier wrote:
>> Warning (smie): Conflict: . </= .
>> Warning (smie): Conflict: . </= .
>
> The smie-precs->prec2 part of your grammar says that "." = "." (because
> of the new (assoc ".") you add there).
>
> But the BNF part of your grammar says "." < "." (i.e. "foo . bar . baz"
> is parsed as "foo . (bar . baz)" because of (id "." exp).
Fantastic, thanks for the explanation. With (assoc ".") changed to
(right "."), the warning is gone.
I've also fixed the precedence of "." vs "? :" and modified a test case
respectively. See the new patch attached.
Do you think it's not too late in the feature freeze to install a change
like this, or should we wait until the trunk reopens?
We have a decent number of test cases, so it's not like the patch is
likely to introduce major breakage.
> Apparently there is no more conflict in the BNF in this respect so the
> disambiguation constraint (assoc ".") that's passed to it is ignored.
Without that constraint there, SMIE showed warnings about ambiguities
between ".", "," and "=". It somehow ignored the associativity, though.
The warnings with the previous patch also were gone when only the second
one (assoc ".") were changed to (right "."). The first one, in the BNF
disambiguation part, could be either.
Bozhidar, would you care to test the new patch?
[-- Attachment #2: ruby-chained-calls.diff --]
[-- Type: text/x-patch, Size: 4299 bytes --]
=== modified file 'lisp/progmodes/ruby-mode.el'
--- lisp/progmodes/ruby-mode.el 2014-01-17 03:15:02 +0000
+++ lisp/progmodes/ruby-mode.el 2014-01-31 03:17:28 +0000
@@ -264,6 +264,13 @@
:safe 'listp
:version "24.4")
+(defcustom ruby-align-chained-calls nil
+ "If non-nil, chained method calls on multiple lines will be
+aligned to the same column."
+ :type 'boolean
+ :group 'ruby
+ :safe 'booleanp)
+
(defcustom ruby-deep-arglist t
"Deep indent lists in parenthesis when non-nil.
Also ignores spaces after parenthesis when `space'.
@@ -350,10 +357,10 @@
;; but avoids lots of conflicts:
(exp "and" exp) (exp "or" exp))
(exp (exp1) (exp "," exp) (exp "=" exp)
- (id " @ " exp)
- (exp "." id))
+ (id " @ " exp))
(exp1 (exp2) (exp2 "?" exp1 ":" exp1))
- (exp2 ("def" insts "end")
+ (exp2 (exp3) (exp3 "." exp2))
+ (exp3 ("def" insts "end")
("begin" insts-rescue-insts "end")
("do" insts "end")
("class" insts "end") ("module" insts "end")
@@ -380,7 +387,7 @@
(ielsei (itheni) (itheni "else" insts))
(if-body (ielsei) (if-body "elsif" if-body)))
'((nonassoc "in") (assoc ";") (right " @ ")
- (assoc ",") (right "=") (assoc "."))
+ (assoc ",") (right "="))
'((assoc "when"))
'((assoc "elsif"))
'((assoc "rescue" "ensure"))
@@ -399,7 +406,8 @@
(nonassoc ">" ">=" "<" "<=")
(nonassoc "==" "===" "!=")
(nonassoc "=~" "!~")
- (left "<<" ">>"))))))
+ (left "<<" ">>")
+ (right "."))))))
(defun ruby-smie--bosp ()
(save-excursion (skip-chars-backward " \t")
@@ -609,7 +617,18 @@
;; When after `.', let's always de-indent,
;; because when `.' is inside the line, the
;; additional indentation from it looks out of place.
- ((smie-rule-parent-p ".") (smie-rule-parent (- ruby-indent-level)))
+ ((smie-rule-parent-p ".")
+ (let (smie--parent)
+ (save-excursion
+ ;; Traverse up the parents until the parent is "." at
+ ;; indentation, or any other token.
+ (while (and (progn
+ (goto-char (1- (cadr (smie-indent--parent))))
+ (not (ruby-smie--bosp)))
+ (progn
+ (setq smie--parent nil)
+ (smie-rule-parent-p "."))))
+ (smie-rule-parent))))
(t (smie-rule-parent))))))
(`(:after . ,(or `"(" "[" "{"))
;; FIXME: Shouldn't this be the default behavior of
@@ -622,7 +641,10 @@
(unless (or (eolp) (forward-comment 1))
(cons 'column (current-column)))))
(`(:before . "do") (ruby-smie--indent-to-stmt))
- (`(:before . ".") ruby-indent-level)
+ (`(:before . ".")
+ (if (smie-rule-sibling-p)
+ (and ruby-align-chained-calls 0)
+ ruby-indent-level))
(`(:after . "=>") ruby-indent-level)
(`(:before . ,(or `"else" `"then" `"elsif" `"rescue" `"ensure"))
(smie-rule-parent))
=== modified file 'test/automated/ruby-mode-tests.el'
--- test/automated/ruby-mode-tests.el 2014-01-01 07:43:34 +0000
+++ test/automated/ruby-mode-tests.el 2014-01-30 04:15:02 +0000
@@ -333,6 +333,20 @@
| 42
| end")))
+(ert-deftest ruby-align-chained-calls ()
+ (let ((ruby-align-chained-calls t))
+ (ruby-should-indent-buffer
+ "one.two.three
+ | .four
+ |
+ |my_array.select { |str| str.size > 5 }
+ | .map { |str| str.downcase }"
+ "one.two.three
+ | .four
+ |
+ |my_array.select { |str| str.size > 5 }
+ | .map { |str| str.downcase }")))
+
(ert-deftest ruby-move-to-block-stops-at-indentation ()
(ruby-with-temp-buffer "def f\nend"
(beginning-of-line)
=== modified file 'test/indent/ruby.rb'
--- test/indent/ruby.rb 2014-01-17 03:15:02 +0000
+++ test/indent/ruby.rb 2014-01-31 02:55:31 +0000
@@ -257,8 +257,8 @@
bar
foo_bar_tee(1, 2, 3)
- .qux
- .bar
+ .qux.bar
+ .tee
foo do
bar
@@ -338,7 +338,7 @@
%^abc^
ddd
-qux = foo ?
+qux = foo.fee ?
bar :
tee
@@ -348,7 +348,7 @@
zoo
.lose(
- q, p)
+ q, p)
foo(bar:
tee)
next prev parent reply other threads:[~2014-01-31 3:42 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-30 4:24 bug#16593: 24.3.50; ruby-mode: align chained method calls on multiple lines Dmitry Gutov
2014-01-30 14:35 ` Stefan Monnier
2014-01-31 3:42 ` Dmitry Gutov [this message]
2014-01-31 13:31 ` Bozhidar Batsov
2014-01-31 17:04 ` Dmitry Gutov
2014-01-31 17:18 ` Glenn Morris
2014-01-31 17:33 ` Dmitry Gutov
2014-01-31 14:33 ` Stefan Monnier
2014-01-31 17:34 ` Dmitry Gutov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52EB1B96.4020602@yandex.ru \
--to=dgutov@yandex.ru \
--cc=16593@debbugs.gnu.org \
--cc=bozhidar@batsov.com \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.