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

Previous Next

Full log

Message #14 received at 16078 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Cameron Desautels <camdez <at> gmail.com>
Cc: 16078 <at> debbugs.gnu.org, Glenn Morris <rgm <at> gnu.org>
Subject: Re: bug#16078: Extensive docs and tests for `ruby-forward-string'
 (PATCH)
Date: Mon, 09 Dec 2013 06:07:44 +0200

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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.