bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#16078: Extensive docs and tests for `ruby-forward-string' (PATCH)


From: Dmitry Gutov
Subject: bug#16078: Extensive docs and tests for `ruby-forward-string' (PATCH)
Date: Mon, 09 Dec 2013 06:07:44 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.1

On 08.12.2013 04:29, Cameron Desautels wrote:
Thank you for explaining the recent changes.  I did see a lot of
duplicated parsing code and wasn't completely sure what the reason for
it was.  I read up a bit on SMIE so that makes sense now.

Even without SMIE, there already was duplication between `ruby-syntax-propertize-function' (and its handling of string interpolations and percent literals) and the manual parsing of them in `ruby-parse-partial' and `ruby-forward-string'.

So, if I were going to untangle that code, first I'd have tried to replace all the uses of `ruby-forward-string' with `forward-sexp', and maybe `narrow-to-region', to handle the END parameter. We already know where those literals end, no need to read them char-by-char again.

As a minimal example to recreate the issue, create a new file with the
following contents (indentation removed):

     def foo
       %^bar^
     end

Thank you for the example.

Turn on ruby-mode, and run `imenu-add-menubar-index'.  You will get
the following error:

I've now also fixed it not to call `ruby-parse-partial' when SMIE is used.

This leaves `ruby-move-to-block' as the only function that still requires the old indentation engine to work. Not sure yet how to rewrite it in terms of sexp navigation or other SMIE-specific functions.

I have not signed the copyright assignment papers but I am willing.

For now I've installed everything except the tests. Combined with your patch for #16046, the docstring and the patch in #16079 go over the 15 lines limit, though not far.

If the more experienced contributors say it's too much, we can remove the docstring.

For the tests, though, and any further patches, the assignment will be required.

Glenn, could you send the form?

Speaking of the tests, though, the way they are now, they only cover one parsing/indentation engine. If they were written to be more high-level, say, testing indentation or the IMenu index generation, they could touch both engines, which would be better.

Although if `ruby-forward-string' were replaced with `forward-sexp', there probably would be no need for most of the new tests, since the code would handle all cases uniformly.

N.B.: Your example above also uncovered an indentation bug when using SMIE. Gonna fix that shortly.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]