bug#14457: 24.3; buggy forward-sexp in octave mode?

[Leo Liu]

1. Open the attached file
2. Move point to end of the word 'otherwise'
3. M-: (forward-sexp -1)

Is this correct behaviour? I noticed this via
smie-highlight-matching-block-mode.

Leo

[Stefan Monnier]

> 1. Open the attached file
> 2. Move point to end of the word 'otherwise'
> 3. M-: (forward-sexp -1)

Please in your bug reports, do mention the behavior that you see rather
than assume that I will see the same one. Also, try to explain the
behavior you would have liked to see instead.

What I see is that it jumps to "right after the previous matching case".
Is that what you see?

> Is this correct behaviour?

It's the expected behavior, at least (IOW if it changes, you'll have to
adjust the indentation rules accordingly).

Since "after otherwise" is not a position that corresponds to the end of
a "sexp", the meaning of (forward-sexp -1) is unclear. SMIE defines
such things in a way I found useful, but admittedly, it might take some
time to get used to it.

What behavior did you expect?


Stefan

[Andreas Röhler]

Am 24.05.2013 05:07, schrieb Leo Liu:
> 1. Open the attached file
> 2. Move point to end of the word 'otherwise'
> 3. M-: (forward-sexp -1)
>

> Is this correct behaviour? I noticed this via
> smie-highlight-matching-block-mode.
>
> Leo
>

forward-sexp isn't able to leave a string when called from inside.
Which constitutes a bug.

For example curser at end inside string:

case "importdata"
^
First backward-sexp would reach beginning of string, OK.
Second call fails.

forward-sexp called at beginning or end should go one level up or return nil if BOB/EOB

Andreas

[Leo Liu]

On 2013-05-24 12:07 +0800, Stefan Monnier wrote:
> Please in your bug reports, do mention the behavior that you see rather
> than assume that I will see the same one. Also, try to explain the
> behavior you would have liked to see instead.

Sorry, Stefan, that was an oversight.

>
> What I see is that it jumps to "right after the previous matching
>case". Is that what you see?

It jumps to the previous 'case' with point on the opening '{'.

>
>> Is this correct behaviour?
>
> It's the expected behavior, at least (IOW if it changes, you'll have to
> adjust the indentation rules accordingly).
>
> Since "after otherwise" is not a position that corresponds to the end of
> a "sexp", the meaning of (forward-sexp -1) is unclear. SMIE defines
> such things in a way I found useful, but admittedly, it might take some
> time to get used to it.
>
> What behavior did you expect?

Since 'case', 'otherwise' are closers to 'switch' as in
smie-closer-alist, I was expecting (forward-sexp -1) to jump back to
'switch', much like from 'elseif' to 'if'. Does this make sense?

Leo

[Leo Liu]

On 2013-05-24 13:30 +0800, Andreas Röhler wrote:
> forward-sexp isn't able to leave a string when called from inside.
> Which constitutes a bug.
>
> For example curser at end inside string:
>
> case "importdata"
> ^
> First backward-sexp would reach beginning of string, OK.
> Second call fails.

This is a different issue and I am not sure what to do here. Maybe it
makes more sense to let forward-list and backward-up-list move out of
strings. Feel free to make a proposal.

Leo

[Stefan Monnier]

> Since 'case', 'otherwise' are closers to 'switch' as in
> smie-closer-alist, I was expecting (forward-sexp -1) to jump back to
> 'switch', much like from 'elseif' to 'if'. Does this make sense?

Both behaviors make sense. Note that elseif/else behaves just like
case/otherwise: if will stop at the previous matching elseif.

For indentation purpose it's better if it doesn't jump
too far, which is why octave-mode currently behaves this way.
The reason why it's better is:
- faster indentation since we parse less of the buffer.
- more local decision means that the behavior is easier to understand
for the user.
- also means that it better takes into account choices of the user: if
the user decides to place his "case" at some other indentation, only
the first "case" after "switch" will disagree with the user, all the
other ones will simply align under the first.

Ideally, this behavior would also allow to use C-M-t to transpose two
cases, just like you can do with the usual infix operators/separators,
but currently this doesn't work (and it can't be done with "otherwise").


Stefan

[Andreas Röhler]

Am 25.05.2013 06:20, schrieb Leo Liu:

> On 2013-05-24 13:30 +0800, Andreas R�hler wrote:
>> forward-sexp isn't able to leave a string when called from inside.
>> Which constitutes a bug.
>>
>> For example curser at end inside string:
>>
>> case "importdata"
>> ^
>> First backward-sexp would reach beginning of string, OK.
>> Second call fails.
>
> This is a different issue

That's right. Sorry mixing that up.

and I am not sure what to do here. Maybe it
> makes more sense to let forward-list and backward-up-list move out of
> strings. Feel free to make a proposal.
>
> Leo
>

If backward-sexp is called from inside a string, would assume the string being the balanced expression then.
I.e. it should go to the beginning of string. From there proceed as now.

Andreas

[Leo Liu]

On 2013-05-25 14:59 +0800, Stefan Monnier wrote:
> Both behaviors make sense. Note that elseif/else behaves just like
> case/otherwise: if will stop at the previous matching elseif.
>
> For indentation purpose it's better if it doesn't jump
> too far, which is why octave-mode currently behaves this way.
> The reason why it's better is:
> - faster indentation since we parse less of the buffer.
> - more local decision means that the behavior is easier to understand
> for the user.
> - also means that it better takes into account choices of the user: if
> the user decides to place his "case" at some other indentation, only
> the first "case" after "switch" will disagree with the user, all the
> other ones will simply align under the first.
>
> Ideally, this behavior would also allow to use C-M-t to transpose two
> cases, just like you can do with the usual infix operators/separators,
> but currently this doesn't work (and it can't be done with "otherwise").

I have noticed with your last change to smie, 'case' 'elseif' are now
nicely highlighted. So I agree with your points and consider this done ;)

Leo