Re: [Emacs-diffs] master bb591f139f: Enhance CC Mode's fontification, etc., of unterminated strings.

[Alan Mackenzie]

Hello, João.

On Tue, May 22, 2018 at 20:21:25 +0100, João Távora wrote:
> Hi Alan,

> Alan Mackenzie <address@hidden> writes:

> > electric-tests.el is anything but straightforward to read.

> lol, sorry I couldn't quite make the 20000+LOC standards of cc-mode.el
> :p.

:-)

> No really, kidding, <wipes tears>, that file needs macros because it
> defines almost 500 tests with very subtle variations between them.

Yes, I understand this.

> You tripped one of them, and I think we're both glad you did
> (regardless of who is at fault: test or c++-mode)

Well, I'm glad about it, so thanks for this test file!.  The bug is most
definitely in CC Mode - I'd neglected to handle properly an open string
lacking an EOL to "terminate" it.  This will be easy to fix.

> > The test referred to above is generated by a nest of two or three
> > macros, somehow, and it is not obvious what buffer operations were
> > generated by these macros, and how they triggered a newly introduced bug
> > in C++ Mode.  The comments in the file are too sparse to help.

> Here's how it works: There's only one macro for electric-pair tests,
> aptly named define-electric-pair-test.

> In that file, find the `define-electric-pair-test' that most closely
> matches the test failure, in this case its:

>    (define-electric-pair-test autowrapping-5
>      "foo" "\"" :expected-string "\"foo\"" :expected-point 2
>      :fixture-fn #'(lambda ()
>                      (electric-pair-mode 1)
>                      (mark-sexp 1)))

> now go to the end of the expression and type M-x
> pp-macroexpand-last-sexp. It should be easy to find your failing test,
> defined in terms of `ert-deftest', in a list of 6 tests. Here it is:

>   (ert-deftest electric-pair-autowrapping-5-at-point-2-in-c++-mode-in-strings 
> nil
>   "With |\"foo\"|, try input \" at point 2. Should become |\"\"foo\"\"| and 
> point at 3"
>                (electric-pair-test-for "\"foo\"" 2 34 "\"\"foo\"\"" 3 
> 'c++-mode nil
>                                        #'(lambda nil
>                                            (electric-pair-mode 1)
>                                            (mark-sexp 1))))

> Now M-x edebug-defun this form straight in the *Pp Macroexpand Output*
> buffer, and M-x edebug-defun the `electric-pair-test-for' defun,
> too. Now run the test:

>   M-x ert RET electric-pair-autowrapping-5-at-point-2-in-c++-mode-in-strings 
> RET

Thanks for this recipe.  It wasn't self evident.

> As you step through the code, you'll eventually land on that lambda
> which calls mark-sexp and hints, along with the name, that this is a
> region-autowrapping test. This is why it expects, for a single character
> of input, that two quotes are inserted in the buffer instead of
> one. The test passes in my 26.1 as you probably already knew.

> Good luck hunting the bug and let me know if you have more problems.

Thanks for the help.

> Thanks,
> João

> PS: you could also have used:

>    M-x ert-describe-test RET 
> electric-pair-autowrapping-5-at-point-2-in-c++-mode-in-strings

> Which would have rendered a nice docstring

>     electric-pair-autowrapping-5-at-point-2-in-c++-mode-in-strings is a
>     test defined in `electric-tests.elc'.
     
>     With |"foo"|, try input " at point 2. Should become |""foo""| and point 
> at 3
     
>     [back]

> Though, admittedly, this is misleading for the "autowrapping" tests,
> since it doesn't tell you about the "mark-sexp" region-making
> command. Also not immediatly clear perhaps it that the | are buffer
> boundaries.

Just to be pedantic, it also doesn't say where " number 4 is inserted
(whether at EOL, or before " number 2), but that can be resolved by the
application of intelligence.  :-)

> Ideally, it should read read

>   With |"foo"|, at point 2, (mark-sexp 1) and try input ".
>   Should become |""foo""| and point at 3

> I will try to fix this in master.

> Also M-x ert-find-test-other-window could have helped you, but it
> doesn't (brings me to the beginning of the file, which isn't helpful). I
> don't know why, does anyone?

-- 
Alan Mackenzie (Nuremberg, Germany).