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