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

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

[Leo Liu]

[-- Attachment #1: Type: text/plain, Size: 185 bytes --]

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


[-- Attachment #2: unimplemented.m --]
[-- Type: text/plain, Size: 1742 bytes --]

## -*- mode: octave; -*-

function txt = unimplemented (fcn)

  is_matlab_function = true;

  ## Some smarter cases, add more as needed.
  switch (fcn)

  case "importdata"
    txt = ["importdata is not implemented.  Similar functionality is ",...
    "available through @code{load}, @code{dlmread}, @code{csvread}, ",...
    "or @code{textscan}."];  

  case "quad2d"
    txt = ["quad2d is not implemented.  Consider using dblquad."];

  case "gsvd"
    txt = ["gsvd is not currently part of core Octave.  See the ",...
    "linear-algebra package at ",...
    "@url{http://octave.sourceforge.net/linear-algebra/}."];

  case "linprog"
    txt = ["Octave does not currently provide linprog.  ",...
    "Linear programming problems may be solved using @code{glpk}.  ",...
    "Try @code{help glpk} for more info."];

  case {"ode113", "ode15i", "ode15s", "ode23", "ode23s", "ode23t", "ode45", "odeget", "odeset"}
    txt = ["Octave provides lsode for solving differential equations.  ",...
    "For more information try @code{help lsode}.  ",...
    "Matlab-compatible ODE functions are provided by the odepkg package.  ",...
    "See @url{http://octave.sourceforge.net/odepkg/}."];

  otherwise
    if (ismember (fcn, missing_functions ()))
      txt = sprintf ("the '%s' function is not yet implemented in Octave", fcn);
    else
      is_matlab_function = false;
      txt = "";
    endif
  endswitch

  if (is_matlab_function)
    txt = [txt, "\n\n@noindent\nPlease read ",...
           "@url{http://www.octave.org/missing.html} to learn how ",...
           "you can contribute missing functionality."];
    txt = __makeinfo__ (txt);
  endif

  if (nargout == 0)
    warning ("Octave:missing-function", "%s", txt);
  endif

endfunction

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

[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

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

[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

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

[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

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

[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

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

[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

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

[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

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

[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