bug#22717: 25.0.91; sh-mode / forward-comment bug with opening parens at the beginnings of lines, and quotes in comments

[Phil Sainty]

On 18/02/16 02:18, Phil Sainty wrote:
> I've come up with the following test case:
> 
> #!/bin/sh
> echo "
> (" # Opening parenthesis at the beginning of a line
> 
> if true; then
>     # Here's an apostrophe
> fi # ...and now this will be indented -- aligned with the apostrophe
> 
> 
> If you auto-indent that, the final line gets indented to align
> with the column containing the apostrophe in the preceding comment.


Taking another look at this, I've narrowed it down to the behaviour
of `forward-comment':

With point at the beginning of the "fi" line, running
(forward-comment -1) moves point to the end of the preceding line,
after the word "apostrophe".

If any of the following changes are made, the (forward-comment -1)
call will move point to the "#" at the beginning of the comment:

* Remove the ' from the comment
* Add an additional ' to the comment line (i.e. a balanced quantity)
* Change the line with the opening parenthesis so that the lines does
  not begin with the paren.

I note that adding a closing paren (so they are balanced) does not
make a difference.

The issue occurs with double-quote and back-quote characters as well
as apostrophes. Any of: ' " `

Similarly, we can substitute a square or curly opening parenthesis
for the round opening paren. Any of ( [ {

I've tested this with version 24.5, 25.2rc2, and master.


-Phil


p.s. The actual indentation code is presumably irrelevant, but the
sequence of events for that was:

smie-indent-line calls smie-indent-calculate which runs
smie-indent-functions until a non-nil value is found.

In this instance it is smie-indent-keyword which returns a value,
and the key part is (let* ((parent (smie-backward-sexp token)))

token here is "fi" and calling (smie-backward-sexp "fi") manually with
point at the beginning of that line, we can observe that point moves
to the apostrophe inside the comment.  If we remove the apostrophe,
point moves to the start of the corresponding "if" instead.

smie-backward-sexp returns (t 96 "'") in the (t POS TOKEN) format.

Ultimately it is smie-next-sexp which produces the value, apparently
failing to skip comments during its processing.  The docstring says
"NEXT-TOKEN is a function of no argument that moves forward by one
token (after skipping comments if needed)". NEXT-TOKEN is compiled
elisp produced by (indirect-function smie-backward-token-function)
where smie-backward-token-function is sh-smie-sh-backward-token.

About the first thing to happen in sh-smie-sh-backward-token is
(forward-comment (- (point))) to skip backwards over as many comments
as possible.