* Re: trunk r114534: Get Ruby's SMIE code to pass the test suite. [not found] <E1VSWj3-0005Os-CP@vcs.savannah.gnu.org> @ 2013-10-06 0:20 ` Dmitry Gutov 2013-10-06 16:45 ` Stefan Monnier 0 siblings, 1 reply; 11+ messages in thread From: Dmitry Gutov @ 2013-10-06 0:20 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: > ------------------------------------------------------------ > revno: 114534 > revision-id: monnier@iro.umontreal.ca-20131005183708-jfv7jyagzpkqkljx > parent: dgutov@yandex.ru-20131005172122-cpmqj9dmeknecra6 > committer: Stefan Monnier <monnier@iro.umontreal.ca> > branch nick: trunk > timestamp: Sat 2013-10-05 14:37:08 -0400 > message: > Get Ruby's SMIE code to pass the test suite. > * lisp/progmodes/ruby-mode.el (ruby-use-smie): Change default. Cool stuff. > === modified file 'test/ChangeLog' > --- a/test/ChangeLog 2013-10-04 21:45:37 +0000 > +++ b/test/ChangeLog 2013-10-05 18:37:08 +0000 > @@ -1,3 +1,8 @@ > +2013-10-05 Stefan Monnier <monnier@iro.umontreal.ca> > + > + * indent/ruby.rb: Port a few cases from automated/ruby-mode-tests.el. > + Adjust indentation of continued line to the new SMIE behavior. I'm slightly concerned that this makes those tests "fail" in the default engine. Shouldn't be too much of a problem, though, if SMIE works well. > + > 2013-10-04 Stefan Monnier <monnier@iro.umontreal.ca> > > * automated/completion-tests.el: > > === modified file 'test/indent/ruby.rb' > --- a/test/indent/ruby.rb 2013-09-15 23:42:26 +0000 > +++ b/test/indent/ruby.rb 2013-10-05 18:37:08 +0000 > @@ -1,3 +1,10 @@ > +if something_wrong? # ruby-move-to-block-skips-heredoc > + ActiveSupport::Deprecation.warn(<<-eowarn) > + boo hoo > + end > + eowarn > +end This "port" omits a significant part of the test. Specifically, its automation. Do you intend to eventually remove it from automated/ruby-mode-tests.el? I'd be firmly against that. Otherwise, duplicating examples from there in this file, as a playground of sorts, seems harmless. > +foo = [1, # ruby-deep-indent > + 2] > + > +foo = { # ruby-deep-indent-disabled > + a : b > +} > + > +foo = [ # ruby-deep-indent-disabled > + 1 > +] > + > +foo( # ruby-deep-indent-disabled > + a > +) Being able to handle all the cases above without touching `ruby-deep-indent' is nice. I've been using an advice for `ruby-indent-line' which does that, for quite some time. > # Bug#15369 > MSG = 'Separate every 3 digits in the integer portion of a number' \ > - 'with underscores(_).' > + 'with underscores(_).' This one doesn't work for me. With SMIE, it indents like this: MSG = 'Separate every 3 digits in the integer portion of a number' \ 'with underscores(_).' ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: trunk r114534: Get Ruby's SMIE code to pass the test suite. 2013-10-06 0:20 ` trunk r114534: Get Ruby's SMIE code to pass the test suite Dmitry Gutov @ 2013-10-06 16:45 ` Stefan Monnier 2013-10-06 21:40 ` Dmitry Gutov 0 siblings, 1 reply; 11+ messages in thread From: Stefan Monnier @ 2013-10-06 16:45 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel >> + * indent/ruby.rb: Port a few cases from automated/ruby-mode-tests.el. >> + Adjust indentation of continued line to the new SMIE behavior. > I'm slightly concerned that this makes those tests "fail" in the default > engine. It's only one test. And yes, the two indentation styles are different. I obviously consider SMIE's to be superior, which is why I changed the ruby.rb file, but I haven't bothered to fix the old engine (not "default" any more). > Shouldn't be too much of a problem, though, if SMIE works well. That's the hope, indeed. > automation. Do you intend to eventually remove it from > automated/ruby-mode-tests.el? I'd be firmly against that. Knowing that it's a sensitive issue, I just stay away from ruby-mode-tests.el. > This one doesn't work for me. With SMIE, it indents like this: > MSG = 'Separate every 3 digits in the integer portion of a number' \ > 'with underscores(_).' I installed a fix to smie.el at the same time. The indentation you show is the one you get with the old smie.el. Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: trunk r114534: Get Ruby's SMIE code to pass the test suite. 2013-10-06 16:45 ` Stefan Monnier @ 2013-10-06 21:40 ` Dmitry Gutov 2013-10-07 4:05 ` Stefan Monnier 0 siblings, 1 reply; 11+ messages in thread From: Dmitry Gutov @ 2013-10-06 21:40 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: > It's only one test. If you mean the example for Bug#15369, you're forgetting the ruby-deep-indent-* examples, they can't all pass using the old engine, with the same value of ruby-deep-indent-paren. > old engine (not "default" any more). Indeed. >> automation. Do you intend to eventually remove it from >> automated/ruby-mode-tests.el? I'd be firmly against that. > > Knowing that it's a sensitive issue, I just stay away from > ruby-mode-tests.el. Ok, thank you. >> This one doesn't work for me. With SMIE, it indents like this: > >> MSG = 'Separate every 3 digits in the integer portion of a number' \ >> 'with underscores(_).' > > I installed a fix to smie.el at the same time. The indentation you show > is the one you get with the old smie.el. Right, sorry. My smie.elc was stale. Could you take a look at the examples I added in 114537 and 114538? They're fairly characterisitc of real-life code (except the optional "do", I guess). ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: trunk r114534: Get Ruby's SMIE code to pass the test suite. 2013-10-06 21:40 ` Dmitry Gutov @ 2013-10-07 4:05 ` Stefan Monnier 2013-10-21 6:14 ` Dmitry Gutov 0 siblings, 1 reply; 11+ messages in thread From: Stefan Monnier @ 2013-10-07 4:05 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel > Could you take a look at the examples I added in 114537 and 114538? See my answer patch. You'll see that I only added support for - and +. Other infix operators are easy to add, but I'd rather someone go through the trouble of finding the precedence table (BTW, you could also use the trick used in sml-mode, where the infix operators are not specified in the BNF grammar but only in a precedence table). Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: trunk r114534: Get Ruby's SMIE code to pass the test suite. 2013-10-07 4:05 ` Stefan Monnier @ 2013-10-21 6:14 ` Dmitry Gutov 2013-10-21 12:46 ` Stefan Monnier 0 siblings, 1 reply; 11+ messages in thread From: Dmitry Gutov @ 2013-10-21 6:14 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 07.10.2013 08:05, Stefan Monnier wrote: >> Could you take a look at the examples I added in 114537 and 114538? > > See my answer patch. You'll see that I only added support for - and +. > Other infix operators are easy to add, but I'd rather someone go through > the trouble of finding the precedence table (BTW, you could also use the > trick used in sml-mode, where the infix operators are not specified in > the BNF grammar but only in a precedence table). Thank you, done, in 114732. Please review when you have the time. Some further questions: 1) Is `assoc' any different from `left' in practice? 2) `ruby-smie-grammar' has a FIXME at the top. What kind of Cucumber support do you suppose is missing? The "Given /toto/ do" example in ruby.rb looks fine, mostly due to the syntax-propertize-function, which hasn't changed. 3) Please take a look at the following example: foo_bar_tee(1, 2, 3) .qux What would be the best way to make it work (and also similar example with the dot on the first line)? Add another two cases to `ruby-smie-rules', for the token "."? That wouldn't exactly, I think, work because `ruby-smie--forward-token' and its counterpart like to concatenate the dot with the identifier that goes after it. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: trunk r114534: Get Ruby's SMIE code to pass the test suite. 2013-10-21 6:14 ` Dmitry Gutov @ 2013-10-21 12:46 ` Stefan Monnier 2013-10-21 14:16 ` Dmitry Gutov 0 siblings, 1 reply; 11+ messages in thread From: Stefan Monnier @ 2013-10-21 12:46 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel > 1) Is `assoc' any different from `left' in practice? Yes: with `assoc', when you have "a + b + c", smie first tries to align "c" with "b", whereas with `left' it would skip "b" and go straight to "a". When there are only 3 elements, it rarely makes a difference, but for things like "," or ";" where there can be tens or hundreds of elements, the different can be very significant in terms of indentation speed. Also, it makes a difference if the user purposefully "misindents" some of the elements, of course. > 2) `ruby-smie-grammar' has a FIXME at the top. What kind of Cucumber support > do you suppose is missing? The "Given /toto/ do" example in ruby.rb looks > fine, mostly due to the syntax-propertize-function, which hasn't changed. That fixme migth be out of date, indeed. > 3) Please take a look at the following example: > foo_bar_tee(1, 2, 3) > .qux > What would be the best way to make it work (and also similar example with > the dot on the first line)? > Add another two cases to `ruby-smie-rules', for the token "."? That wouldn't > exactly, I think, work because `ruby-smie--forward-token' and its > counterpart like to concatenate the dot with the identifier that goes > after it. Maybe change the tokenizer so that a ".qux" *at smie-bolp* is tokenized as "." and "qux"? Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: trunk r114534: Get Ruby's SMIE code to pass the test suite. 2013-10-21 12:46 ` Stefan Monnier @ 2013-10-21 14:16 ` Dmitry Gutov 2013-10-21 16:46 ` Stefan Monnier 0 siblings, 1 reply; 11+ messages in thread From: Dmitry Gutov @ 2013-10-21 14:16 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> 1) Is `assoc' any different from `left' in practice? > > Yes: with `assoc', when you have "a + b + c", smie first tries to align > "c" with "b", whereas with `left' it would skip "b" and go straight to > "a". When there are only 3 elements, it rarely makes a difference, but > for things like "," or ";" where there can be tens or hundreds of > elements, the different can be very significant in terms of > indentation speed. Also, it makes a difference if the user purposefully > "misindents" some of the elements, of course. I see. > Maybe change the tokenizer so that a ".qux" *at smie-bolp* is tokenized > as "." and "qux"? That will probably break the following modified example: class C def foo self .end D.new .class end end But I guess we can make the tokenizer both return "." as a separate token and include it in the token that goes after it. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: trunk r114534: Get Ruby's SMIE code to pass the test suite. 2013-10-21 14:16 ` Dmitry Gutov @ 2013-10-21 16:46 ` Stefan Monnier 2013-10-21 22:30 ` Dmitry Gutov 0 siblings, 1 reply; 11+ messages in thread From: Stefan Monnier @ 2013-10-21 16:46 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel >> Maybe change the tokenizer so that a ".qux" *at smie-bolp* is tokenized >> as "." and "qux"? > That will probably break the following modified example: > class C > def foo > self > .end > D.new > .class > end > end Yes, the problem is that the set of desired indentation does not match the structure of a fixed parsing. So, some of the differences need to be handled in ad-hoc ways in the ruby-smie-rules function. E.g. for (:before . ".") we'd have to look at the previous token and if it's of the form "foo.bar", then manually align with the "." of "foo.bar". > But I guess we can make the tokenizer both return "." as a separate > token and include it in the token that goes after it. I'm not sure I understand exactly what you're suggesting, but I'd venture to say that you can't do that. Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: trunk r114534: Get Ruby's SMIE code to pass the test suite. 2013-10-21 16:46 ` Stefan Monnier @ 2013-10-21 22:30 ` Dmitry Gutov 2013-10-22 15:46 ` Stefan Monnier 2013-10-22 15:47 ` Stefan Monnier 0 siblings, 2 replies; 11+ messages in thread From: Dmitry Gutov @ 2013-10-21 22:30 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel On 21.10.2013 20:46, Stefan Monnier wrote: >>> Maybe change the tokenizer so that a ".qux" *at smie-bolp* is tokenized >>> as "." and "qux"? >> That will probably break the following modified example: >> class C >> def foo >> self >> .end >> D.new >> .class >> end >> end > > Yes, the problem is that the set of desired indentation does not match > the structure of a fixed parsing. So, some of the differences need to > be handled in ad-hoc ways in the ruby-smie-rules function. > > E.g. for (:before . ".") we'd have to look at the previous token and if > it's of the form "foo.bar", then manually align with the "." of > "foo.bar". Indentation is not the only problem there. Tokenizing ".end" as [".", "end"] also breaks sexp movement and (for example) `smie--matching-block-data'. The dot concatenation logic is the result of you fixing that in revision 114545. >> But I guess we can make the tokenizer both return "." as a separate >> token and include it in the token that goes after it. > > I'm not sure I understand exactly what you're suggesting, but I'd > venture to say that you can't do that. I don't see why not. I've installed a patch along these lines in 114738, and it fixes the existing examples. Unfortunately, it broke the more complex situations: foo do bar .tee end def bar foo .baz end ...apparently because (goto-char (cadr (smie-indent--parent))), when called before "." in these examples, brings point to the end of the first line, and so (smie-rule-parent ruby-indent-level) returns a surprising result. Any suggestions what to do about that? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: trunk r114534: Get Ruby's SMIE code to pass the test suite. 2013-10-21 22:30 ` Dmitry Gutov @ 2013-10-22 15:46 ` Stefan Monnier 2013-10-22 15:47 ` Stefan Monnier 1 sibling, 0 replies; 11+ messages in thread From: Stefan Monnier @ 2013-10-22 15:46 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel > I don't see why not. I've installed a patch along these lines in 114738, and > it fixes the existing examples. Unfortunately, it broke the more complex > situations: > foo do > bar > .tee > end > def bar > foo > .baz > end Oh, that was a mistake of mine: the `(:after . ";") should have been `(:before . ";"). I installed that patch in trunk. Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: trunk r114534: Get Ruby's SMIE code to pass the test suite. 2013-10-21 22:30 ` Dmitry Gutov 2013-10-22 15:46 ` Stefan Monnier @ 2013-10-22 15:47 ` Stefan Monnier 1 sibling, 0 replies; 11+ messages in thread From: Stefan Monnier @ 2013-10-22 15:47 UTC (permalink / raw) To: Dmitry Gutov; +Cc: emacs-devel > Indentation is not the only problem there. Tokenizing ".end" as [".", "end"] I wasn't suggesting that. We'd turn ".end" into something like [".", ".end"] or [".", "identifier"]. Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-10-22 15:47 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <E1VSWj3-0005Os-CP@vcs.savannah.gnu.org> 2013-10-06 0:20 ` trunk r114534: Get Ruby's SMIE code to pass the test suite Dmitry Gutov 2013-10-06 16:45 ` Stefan Monnier 2013-10-06 21:40 ` Dmitry Gutov 2013-10-07 4:05 ` Stefan Monnier 2013-10-21 6:14 ` Dmitry Gutov 2013-10-21 12:46 ` Stefan Monnier 2013-10-21 14:16 ` Dmitry Gutov 2013-10-21 16:46 ` Stefan Monnier 2013-10-21 22:30 ` Dmitry Gutov 2013-10-22 15:46 ` Stefan Monnier 2013-10-22 15:47 ` Stefan Monnier
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.