* bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t @ 2018-08-22 11:36 Artur Malabarba 2018-08-22 12:50 ` Dmitry Gutov 2020-09-11 17:16 ` Wendel Scardua 0 siblings, 2 replies; 10+ messages in thread From: Artur Malabarba @ 2018-08-22 11:36 UTC (permalink / raw) To: 32496 [-- Attachment #1: Type: text/plain, Size: 512 bytes --] 1. (setq ruby-align-chained-calls t) 2. (setq ruby-use-smie t) 3. Open a file in ruby-mode, insert the following and indent it ---------- some_variable.where.not(x: nil) .where(y: 2) ---------- Expected behaviour: Nothing would happen, the code is already properly indented. What actually happens: The code gets indented as follows ---------- some_variable.where.not(x: nil) .where(y: 2) ---------- Note that this is conflicts with the indentation enforced by rubocop. Artur [-- Attachment #2: Type: text/html, Size: 780 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t 2018-08-22 11:36 bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t Artur Malabarba @ 2018-08-22 12:50 ` Dmitry Gutov 2018-10-27 22:22 ` Artur Malabarba 2020-09-11 17:16 ` Wendel Scardua 1 sibling, 1 reply; 10+ messages in thread From: Dmitry Gutov @ 2018-08-22 12:50 UTC (permalink / raw) To: bruce.connor.am, 32496 On 8/22/18 2:36 PM, Artur Malabarba wrote: > 1. (setq ruby-align-chained-calls t) > 2. (setq ruby-use-smie t) > 3. Open a file in ruby-mode, insert the following and indent it > > ---------- > some_variable.where.not(x: nil) > .where(y: 2) > ---------- > > Expected behaviour: Nothing would happen, the code is already properly > indented. > > What actually happens: The code gets indented as follows > > ---------- > some_variable.where.not(x: nil) > .where(y: 2) > ---------- > > Note that this is conflicts with the indentation enforced by rubocop. I'd like to point out that this is exactly the behavior Bozhidar asked for, back when this variable was introduced. See: http://lists.gnu.org/archive/html/emacs-devel/2014-01/msg01802.html and in particular the Example 1 in the referenced comment: https://github.com/rubocop-hq/ruby-style-guide/pull/176#issuecomment-18664622 So we even have a test (ruby-align-chained-calls) that check that the alignment is do to the last dot, and not to the first one. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t 2018-08-22 12:50 ` Dmitry Gutov @ 2018-10-27 22:22 ` Artur Malabarba 2018-11-18 8:36 ` Bozhidar Batsov 0 siblings, 1 reply; 10+ messages in thread From: Artur Malabarba @ 2018-10-27 22:22 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 32496, Bozhidar Batsov [-- Attachment #1: Type: text/plain, Size: 1669 bytes --] IIUC, bozhidar was requesting that the dots be aligned to the dot above (as opposed to being indented by only 2 spaces). He didn't say what should happen if one of the lines has multiple dots in it. The linked github comment does explicitly request the aligning to the last dot, but it's the only comment that requests that on a very long discussion that was largely focused on a different topic (whether or not to use trailing dots). Bozhidar, do you have an opinion on this? Artur On Wed, 22 Aug 2018 at 09:50, Dmitry Gutov <dgutov@yandex.ru> wrote: > On 8/22/18 2:36 PM, Artur Malabarba wrote: > > 1. (setq ruby-align-chained-calls t) > > 2. (setq ruby-use-smie t) > > 3. Open a file in ruby-mode, insert the following and indent it > > > > ---------- > > some_variable.where.not(x: nil) > > .where(y: 2) > > ---------- > > > > Expected behaviour: Nothing would happen, the code is already properly > > indented. > > > > What actually happens: The code gets indented as follows > > > > ---------- > > some_variable.where.not(x: nil) > > .where(y: 2) > > ---------- > > > > Note that this is conflicts with the indentation enforced by rubocop. > > I'd like to point out that this is exactly the behavior Bozhidar asked > for, back when this variable was introduced. See: > > http://lists.gnu.org/archive/html/emacs-devel/2014-01/msg01802.html > > and in particular the Example 1 in the referenced comment: > > > https://github.com/rubocop-hq/ruby-style-guide/pull/176#issuecomment-18664622 > > So we even have a test (ruby-align-chained-calls) that check that the > alignment is do to the last dot, and not to the first one. > [-- Attachment #2: Type: text/html, Size: 2524 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t 2018-10-27 22:22 ` Artur Malabarba @ 2018-11-18 8:36 ` Bozhidar Batsov 2021-09-01 9:53 ` Lars Ingebrigtsen 0 siblings, 1 reply; 10+ messages in thread From: Bozhidar Batsov @ 2018-11-18 8:36 UTC (permalink / raw) To: Artur Malabarba; +Cc: 32496, Dmitry Gutov [-- Attachment #1: Type: text/plain, Size: 2322 bytes --] Sorry for the radio silence - I've been super busy lately. It's hard for me to understand the indentation in the examples in the email (as it seems the same to me). Very simply put - the idea is to align multi-line chained calls on the `.`, as opposed to just nest them under the root receiver as we'd normally do. I think Dmitry implemented this great and it's behaving just as it's supposed to be behaving. Perhaps you misunderstood how this was supposed to behave? What's the indentation you expected? On Sun, 28 Oct 2018 at 00:22, Artur Malabarba <bruce.connor.am@gmail.com> wrote: > IIUC, bozhidar was requesting that the dots be aligned to the dot above > (as opposed to being indented by only 2 spaces). He didn't say what should > happen if one of the lines has multiple dots in it. > > The linked github comment does explicitly request the aligning to the last > dot, but it's the only comment that requests that on a very long discussion > that was largely focused on a different topic (whether or not to use > trailing dots). > > Bozhidar, do you have an opinion on this? > > Artur > > > On Wed, 22 Aug 2018 at 09:50, Dmitry Gutov <dgutov@yandex.ru> wrote: > >> On 8/22/18 2:36 PM, Artur Malabarba wrote: >> > 1. (setq ruby-align-chained-calls t) >> > 2. (setq ruby-use-smie t) >> > 3. Open a file in ruby-mode, insert the following and indent it >> > >> > ---------- >> > some_variable.where.not(x: nil) >> > .where(y: 2) >> > ---------- >> > >> > Expected behaviour: Nothing would happen, the code is already properly >> > indented. >> > >> > What actually happens: The code gets indented as follows >> > >> > ---------- >> > some_variable.where.not(x: nil) >> > .where(y: 2) >> > ---------- >> > >> > Note that this is conflicts with the indentation enforced by rubocop. >> >> I'd like to point out that this is exactly the behavior Bozhidar asked >> for, back when this variable was introduced. See: >> >> http://lists.gnu.org/archive/html/emacs-devel/2014-01/msg01802.html >> >> and in particular the Example 1 in the referenced comment: >> >> >> https://github.com/rubocop-hq/ruby-style-guide/pull/176#issuecomment-18664622 >> >> So we even have a test (ruby-align-chained-calls) that check that the >> alignment is do to the last dot, and not to the first one. >> > [-- Attachment #2: Type: text/html, Size: 3474 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t 2018-11-18 8:36 ` Bozhidar Batsov @ 2021-09-01 9:53 ` Lars Ingebrigtsen 2021-09-01 10:02 ` Bozhidar Batsov 0 siblings, 1 reply; 10+ messages in thread From: Lars Ingebrigtsen @ 2021-09-01 9:53 UTC (permalink / raw) To: Bozhidar Batsov; +Cc: 32496, Artur Malabarba, Dmitry Gutov Bozhidar Batsov <bozhidar@batsov.com> writes: > Sorry for the radio silence - I've been super busy lately. > > It's hard for me to understand the indentation in the examples in the email > (as it seems the same to me). Very simply put - the idea is to align multi-line > chained calls on the `.`, as opposed to just nest them under the root receiver > as we'd normally do. > > I think Dmitry implemented this great and it's behaving just as it's supposed > to be behaving. Perhaps you misunderstood how this was supposed to > behave? What's the indentation you expected? (I'm going through old bug reports that unfortunately weren't resolved at the time.) The examples were in HTML mail, so they were difficult to understand. Emacs (with (setq ruby-align-chained-calls t)) currently aligns like this: some_variable.where .not(x: nil) .where(y: 2) Which is correct. However, when there's a mixture of keeping things on one line and breaking, we get this: some_variable.where.not(x: nil) .where(y: 2) I think the bug reporter wants: some_variable.where.not(x: nil) .where(y: 2) I.e., align multiline chained calls on the first dot, not the last? (I don't know Ruby, so I have no opinion here.) -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t 2021-09-01 9:53 ` Lars Ingebrigtsen @ 2021-09-01 10:02 ` Bozhidar Batsov 2021-09-02 6:55 ` Lars Ingebrigtsen 0 siblings, 1 reply; 10+ messages in thread From: Bozhidar Batsov @ 2021-09-01 10:02 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 32496, Artur Malabarba, Dmitry Gutov [-- Attachment #1: Type: text/plain, Size: 2127 bytes --] Ah, I finally understood the issue at hand! It's really hard to discuss indentation problems in e-mail. :D Yeah, I can confirm there's a bug when using (setq ruby-align-chained-calls t) in this example: some_variable.where.not(x: nil) .where(y: 2) The two `where`s should be lined up, but currently the second `where` is lined up with the `not`. some_variable.where.not(x: nil) .where(y: 2) I can also confirm that the first behavior is what RuboCop (Ruby's popular linter/formatter) expects, when configured for aligned chained method calls. On Wed, Sep 1, 2021, at 12:53 PM, Lars Ingebrigtsen wrote: > Bozhidar Batsov <bozhidar@batsov.com> writes: > > > Sorry for the radio silence - I've been super busy lately. > > > > It's hard for me to understand the indentation in the examples in the email > > (as it seems the same to me). Very simply put - the idea is to align multi-line > > chained calls on the `.`, as opposed to just nest them under the root receiver > > as we'd normally do. > > > > I think Dmitry implemented this great and it's behaving just as it's supposed > > to be behaving. Perhaps you misunderstood how this was supposed to > > behave? What's the indentation you expected? > > (I'm going through old bug reports that unfortunately weren't resolved > at the time.) > > The examples were in HTML mail, so they were difficult to understand. > > Emacs (with (setq ruby-align-chained-calls t)) currently aligns like > this: > > some_variable.where > .not(x: nil) > .where(y: 2) > > Which is correct. However, when there's a mixture of keeping things on > one line and breaking, we get this: > > some_variable.where.not(x: nil) > .where(y: 2) > > I think the bug reporter wants: > > some_variable.where.not(x: nil) > .where(y: 2) > > I.e., align multiline chained calls on the first dot, not the last? > > (I don't know Ruby, so I have no opinion here.) > > -- > (domestic pets only, the antidote for overdose, milk.) > bloggy blog: http://lars.ingebrigtsen.no > [-- Attachment #2: Type: text/html, Size: 3655 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t 2021-09-01 10:02 ` Bozhidar Batsov @ 2021-09-02 6:55 ` Lars Ingebrigtsen 2021-09-08 19:01 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 10+ messages in thread From: Lars Ingebrigtsen @ 2021-09-02 6:55 UTC (permalink / raw) To: Bozhidar Batsov; +Cc: 32496, Stefan Monnier, Artur Malabarba, Dmitry Gutov "Bozhidar Batsov" <bozhidar@batsov.dev> writes: > Ah, I finally understood the issue at hand! It's really hard to discuss > indentation problems in e-mail. :D > > Yeah, I can confirm there's a bug when using (setq ruby-align-chained-calls t) > in this example: > > some_variable.where.not(x: nil) > .where(y: 2) > > The two `where`s should be lined up, but currently the second `where` is > lined up with the `not`. So this is coming from: ('(:before . ".") (if (smie-rule-sibling-p) (and ruby-align-chained-calls 0) (smie-backward-sexp ".") (cons 'column (+ (current-column) ruby-indent-level)))) In the aligned case, I think we should back up to the first "." in the chain and use that as the column? But my SMIE-fu is pretty much non-existent, so I've added Stefan to the CCs. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t 2021-09-02 6:55 ` Lars Ingebrigtsen @ 2021-09-08 19:01 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-09 14:25 ` Lars Ingebrigtsen 0 siblings, 1 reply; 10+ messages in thread From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-08 19:01 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 32496, Bozhidar Batsov, Artur Malabarba, Dmitry Gutov Lars Ingebrigtsen [2021-09-02 08:55:50] wrote: > "Bozhidar Batsov" <bozhidar@batsov.dev> writes: > >> Ah, I finally understood the issue at hand! It's really hard to discuss >> indentation problems in e-mail. :D >> >> Yeah, I can confirm there's a bug when using (setq ruby-align-chained-calls t) >> in this example: >> >> some_variable.where.not(x: nil) >> .where(y: 2) >> >> The two `where`s should be lined up, but currently the second `where` is >> lined up with the `not`. > > So this is coming from: > > ('(:before . ".") > (if (smie-rule-sibling-p) > (and ruby-align-chained-calls 0) > (smie-backward-sexp ".") > (cons 'column (+ (current-column) > ruby-indent-level)))) > > In the aligned case, I think we should back up to the first "." in the > chain and use that as the column? But my SMIE-fu is pretty much > non-existent, so I've added Stefan to the CCs. You could try something like diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el index c09f007a5ee..c681800f6a7 100644 --- a/lisp/progmodes/ruby-mode.el +++ b/lisp/progmodes/ruby-mode.el @@ -640,7 +640,15 @@ ruby-smie-rules ('(:before . "do") (ruby-smie--indent-to-stmt)) ('(:before . ".") (if (smie-rule-sibling-p) - (and ruby-align-chained-calls 0) + (when ruby-align-chained-calls + (while + (let ((pos (point)) + (parent (smie-backward-sexp "."))) + (if (not (equal (nth 2 parent) ".")) + (progn (goto-char pos) nil) + (goto-char (nth 1 parent)) + (not (smie-rule-bolp))))) + (cons 'column (current-column))) (smie-backward-sexp ".") (cons 'column (+ (current-column) ruby-indent-level)))) @@ -826,13 +834,6 @@ ruby--electric-indent-p ;; FIXME: Remove this? It's unused here, but some redefinitions of ;; `ruby-calculate-indent' in user init files still call it. -(defun ruby-current-indentation () - "Return the indentation level of current line." - (save-excursion - (beginning-of-line) - (back-to-indentation) - (current-column))) - (defun ruby-indent-line (&optional ignored) "Correct the indentation of the current Ruby line." (interactive) But please add corresponding regression tests, Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t 2021-09-08 19:01 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-09 14:25 ` Lars Ingebrigtsen 0 siblings, 0 replies; 10+ messages in thread From: Lars Ingebrigtsen @ 2021-09-09 14:25 UTC (permalink / raw) To: Stefan Monnier; +Cc: 32496, Bozhidar Batsov, Artur Malabarba, Dmitry Gutov Stefan Monnier <monnier@iro.umontreal.ca> writes: > You could try something like [...] > But please add corresponding regression tests, Thanks; seems to work perfectly. (And I've added some more tests, but this one was already being tested, I found out afterwards.) -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t 2018-08-22 11:36 bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t Artur Malabarba 2018-08-22 12:50 ` Dmitry Gutov @ 2020-09-11 17:16 ` Wendel Scardua 1 sibling, 0 replies; 10+ messages in thread From: Wendel Scardua @ 2020-09-11 17:16 UTC (permalink / raw) To: 32496 [-- Attachment #1: Type: text/plain, Size: 489 bytes --] I was about to open a bug about this same issue. Seeing this thread, there seems to have been some misunderstanding between the parts. Bhozidar thought it was about indenting on the dot vs indenting according to the previous line, and in that aspect the feature is working correctly. But when there are multiple dots on a line, their style guide enforces indenting on the *first* dot, not the last, while (setq ruby-align-chained-calls t) - supposedly based on it - enforces the latter. [-- Attachment #2: Type: text/html, Size: 576 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-09-09 14:25 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-22 11:36 bug#32496: 27.0.50; Strange indentation when ruby-align-chained-calls is t Artur Malabarba 2018-08-22 12:50 ` Dmitry Gutov 2018-10-27 22:22 ` Artur Malabarba 2018-11-18 8:36 ` Bozhidar Batsov 2021-09-01 9:53 ` Lars Ingebrigtsen 2021-09-01 10:02 ` Bozhidar Batsov 2021-09-02 6:55 ` Lars Ingebrigtsen 2021-09-08 19:01 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-09 14:25 ` Lars Ingebrigtsen 2020-09-11 17:16 ` Wendel Scardua
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).