bugGNU Octave - Bugs: bug #58323, '...' continuation marker causes...

 
 

bug #58323: '...' continuation marker causes parse error when executed with "run selection" in GUI editor

Submitted by:  Muhali <muhali>
Submitted on:  Thu 07 May 2020 01:45:29 PM UTC  
 
Category:  GUI Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Ready For Test Assigned to:  Torsten Lilge <ttl>
Originator Name:  Open/Closed:  Open
Release:  6.0.90 Operating System:  Any

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Sat 23 May 2020 06:17:59 PM UTC, comment #40: 

The remaining question merely covers a cosmetic aspect. Currently, begin and end of the commands resulting from running selected editor code are marked by comment lines in the history.
Rik, maybe you can share your opinion here, too.  

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sat 23 May 2020 04:14:26 PM UTC, comment #39: 

Okay to close?

Rik <rik5>
Project Administrator
Sun 17 May 2020 10:55:46 AM UTC, comment #38: 

I opened bug #58382 for another issue with the current approach for "Run Selection".

Markus Mützel <mmuetzel>
Project Member
Sat 16 May 2020 10:12:54 AM UTC, comment #37: 

Markus, thanks for the feedback. I am definitely not insisting on the additional comment lines, they are just a way to stuck block F9-lines, since part of them might not have been executed. Depending on the overall opinion, I have no problems with removing them.

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sat 16 May 2020 09:52:11 AM UTC, comment #36: 

Imho, the changes made in this bug resolve the original issue.
Not sure if the majority will like the additional lines surrounding F9-ed code because it might clutter the history. But like I already wrote, I can live with them.

Bug #58370 looks promising to improve the user experience further. No need to have a "Copy selection to console" feature imho if that change should be accepted. Instead it might make sense to use that feature for yet another replacement of the current F9 code. But that is probably better handled in a new bug report.

Markus Mützel <mmuetzel>
Project Member
Fri 15 May 2020 06:55:11 PM UTC, comment #35: 

Coming back to the original issue of this report: Is the current F9-behavior what is commonly expected?

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Thu 14 May 2020 10:21:13 PM UTC, comment #34: 

I just filed bug #58370. If/when that bug is resolved, an entire block of commands, with lines separated by newlines, could be sent to the command line for further editing before pressing Enter to ship it to the lexer/parser. I'm not sure if that's what people are looking for on this bug report, but it's something I'd like to have working for other non-GUI uses.

Mike Miller <mtmiller>
Project Administrator
Thu 14 May 2020 10:03:18 PM UTC, comment #33: 

PMFJI
I'm not sure I follow how "copy selection to console" is supposed to work. Any line followed by a newline is executed right away isn't it? (unless of course open if/for/do while blocks etc.)
So copying a selection comprising several lines w/o executing the complete ones won't work I suppose? only the last one won't be executed.

Or do you mean just copy one line, or a part of it, from the editor? That does work, I often do that those using Ctrl-C/Ctrl-V.
If some F9-lookalike could do that it would be quite handy.

Philip Nienhuis <philipnienhuis>
Project Member
Wed 13 May 2020 07:34:01 PM UTC, comment #32: 

With the previous "F9" behavior the selected code was executed line by line in the console. A keyboard-command within the selection had no effect, i.e., all following commands were still executed. That was the main reason for the change.

I think that "Copy selection to console" might be an additional feature if you do not want to run the code immediately but to have it in the console window for extending and then executing.

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Tue 12 May 2020 07:22:21 AM UTC, comment #31: 

Would the "Copy selection to console" feature be the same as the behavior of the "F9" code before the refactoring?
I don't remember why it was changed in the first place. (I'm sure there was a reason for it.)
What would be the downside of using a "Copy selection to console" command as opposed to "Run selection in temporary file"?

Markus Mützel <mmuetzel>
Project Member
Mon 11 May 2020 04:22:47 PM UTC, comment #30: 

If we want to have the functionality of the use case in comment #28, i.e., extending the selected code, wouldn't it be better to have a separate "Copy selection into console"? When using F9, you are only able to extend the code when it is an unfinished sequence.

@Markus: You are right, the current design throws an error, since the temp. file has a syntax error.

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Mon 11 May 2020 10:25:06 AM UTC, comment #29: 

The behavior described in comment #28 should be possible using the new push parser interface, but will probably require a significantly different approach to parsing user in put than we currently have in the GUI.

John W. Eaton <jwe>
Project Administrator
Mon 11 May 2020 06:45:18 AM UTC, comment #28: 

There might be another issue with the current design:
Assume the following code snippet:

for i=1:3
  disp(i)
end

In Matlab, I can mark the first two lines and "F9" them. Then at the command prompt, I would be able to enter any command (e.g. "a=3"). After that, I can mark the last line of the above code snippet and "F9" it.
The result would be the following:

>> for i=1:3
  disp(i)
a=3
end
     1


a =

     3

     2


a =

     3

     3


a =

     3

I haven't tried if that would work in Octave (but I think it wouldn't).
I also don't know if this is a use case we need to support. What do you think?

Markus Mützel <mmuetzel>
Project Member
Sun 10 May 2020 07:31:48 PM UTC, comment #27: 

My suggestion in comment #26 does not work, since the list view is regularly updated from the history and entries only added to the list view are disappearing.

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sun 10 May 2020 06:16:28 PM UTC, comment #26: 

Some kind of indentation is also a good idea but might be misleading when running selected code repeatedly. Another possibility is to just add the begin/end markers to the gui's history list view and not to octave's history file.   

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sun 10 May 2020 03:20:03 PM UTC, comment #25: 

Thanks, Torsten.

The new behavior is more consistent imho.

I'm not sure I like the additional lines surrounding the selected code in the command history (or command window fwiw) because I have to press the up arrow key more often to get to previous commands.
I'd prefer something more subtle. Maybe prepending two spaces to the front of each line in an F9 block that is added to the history?
But that comes down to personal preference and I can get used to the new behavior.

Markus Mützel <mmuetzel>
Project Member
Sun 10 May 2020 02:20:04 PM UTC, comment #24: 

PS for comment #23: ... although I am still not convinced that lines are added to the history, that are never executed.

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sun 10 May 2020 02:16:47 PM UTC, comment #23: 

New changeset http://hg.savannah.gnu.org/hgweb/octave/rev/777eb315b004 where the F9'ed code is added to the history before it is evaluated and where begin/end selection comments are added to the code display and history.

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sun 10 May 2020 12:42:03 PM UTC, comment #22: 

The first case was entering the commands at the command prompt (as opposed to "F9'ing"). Both lines do show in the command history for me in this case.
If they don't show in the command history when they are "F9'ed", this would be another reason for changing something in the current "F9'ing" code.

And "yes" to your question wrt the second case: The F9-commands are sent first and should appear first in the history.

Markus Mützel <mmuetzel>
Project Member
Sun 10 May 2020 11:46:50 AM UTC, comment #21: 

In cases like the first one, nothing would happen, because sourcing the temp. file would lead to a syntax error. In a case like

a = sum(ones(3,1)
); b=p;

with p undefined, the second line throwing the error is included in the history.

In the second case, showing the commands entered at the debug prompt after the commands issued by F9 in the history would also result in a wrong order between parts of the F9-commands and commands from the debug prompt. Or do you mean that the F9-commands are all sent to the interpreter at first and should therefore all be first in the history?

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sun 10 May 2020 11:15:36 AM UTC, comment #20: 

Thanks for the changes. Imho this looks better now.

Thinking about it again, the way Matlab is doing it ("history" before the rest) makes actually a lot more sense to me.
For a reference assume that you are entering the following at the command prompt:

>> a = sum(one(3,1)
; b=5;
error: parse error:

  syntax error

>>> ; b=5;
    ^

The command in the second line didn't execute. Yet it appears in the command history (which is a good thing imho). So it wouldn't be anything out of the ordinary if commands that never executed appear in the command history.

Then assume that you are F9'ing the following lines:

a = 5
b = 1
a
keyboard
3
b

Maybe the code is more complicated and you added the keyboard statement to manually do a bunch of tests and calculations before you eventually continue execution with "dbcont".
I haven't tried the patch yet. But from what you were writing, I guess that the F9'ed code would be added to the command history after that session.
In this case, the code that would appear last in the command history is not the code that was sent to the interpreter last.
Imho, Matlab got it the right way round.

Markus Mützel <mmuetzel>
Project Member
Sun 10 May 2020 09:33:05 AM UTC, comment #19: 

I have pushed cset http://hg.savannah.gnu.org/hgweb/octave/rev/12ad6f1dc3b6 which implements blockwise "disp, eval, history", where "eval" is before "history" for only adding lines before a possible error to the history.

Markus, thanks again for testing in Matlab and giving the impulse for rethinking the line-by-line approach.

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sat 09 May 2020 07:07:07 PM UTC, comment #18: 

Here is the output in the command window when replacing the "error" from comment #16 with keyboard:

>> a = 5
b = 1
a
keyboard
3
b

a =

     5


b =

     1


a =

     5

K>> dbcont

ans =

     3


b =

     1

The "dbcont" that I typed at the prompt is shown after the command block in the history and is not part of the group.

So it looks like they really just do "disp, history, eval" (or "history, disp, eval") and let the interpreter continue from there.

Markus Mützel <mmuetzel>
Project Member
Sat 09 May 2020 06:54:59 PM UTC, comment #17: 

Ah, okay. AFAIK, octave's history is not able to depict groups and errors. What happens when "keyboard" is included in the selected code and some commands are entered at the debug prompt? Are they shown before, after or within the "F9-history"?

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sat 09 May 2020 04:44:11 PM UTC, comment #16: 

To give them more credit: For a visual clue, they add a bracket in the command history next to commands that have been executed in a group and indicate with a red minus if a command or group failed. See the attached screenshot.

Not sure if our history would be able to tell if commands were grouped together (or failed).


Markus Mützel <mmuetzel>
Project Member
Sat 09 May 2020 04:17:22 PM UTC, comment #15: 

Markus, thank you very much for testing.

Indeed, just copying Matlab's behavior would really be much easier (disp, eval, hist). But having commands in the history that were never executed sounds strange. I will see, how this would make the new solution more complicated.

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sat 09 May 2020 03:38:14 PM UTC, comment #14: 

Also, if the code contains an error, Matlab doesn't bother to include only the lines that executed successfully to the history but just adds everything that was F9'ed.
That would also be good enough imho. And sounds less complicated.

Markus Mützel <mmuetzel>
Project Member
Sat 09 May 2020 03:35:45 PM UTC, comment #13: 

That sounds good to me.
Fwiw Matlab's output for F9'ing the following commands

a = 5
b = 1
a
3
b

looks like this:

>> a = 5
b = 1
a
3
b

a =

     5


b =

     1


a =

     5


ans =

     3


b =

     1

So it is basically a "disp" followed by an "eval" for the whole block.

But we don't have to follow exactly. What you outlined might be just as good.

Markus Mützel <mmuetzel>
Project Member
Sat 09 May 2020 03:27:15 PM UTC, comment #12: 

This might be a compromise: Adding all lines to the history after executing the block as a whole or in case of an error the lines up to a possible error.

Displaying the lines is more complicated, not because of a possible prompt but due to the requirement that the command should be displayed when the command is executed. Otherwise, "inputs" and
 "outputs" were mixed in the console window. But if you think that the prompt is not important, I can try to use "echo function on" to get what is required.

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sat 09 May 2020 01:32:44 PM UTC, comment #11: 

So to have the best of both worlds: Would it be possible to execute the commands as a block but add them to the history line by line?

I'm not sure how appending to the history works though.

Also I don't think it is necessary to prepend PS1 in front of every command when several are run as a block. Fwiw, Matlab doesn't bother to insert the ">>" in this case either.

Markus Mützel <mmuetzel>
Project Member
Sat 09 May 2020 11:54:24 AM UTC, comment #10: 

Actually, the very first approach wlas like that, i.e. copying the commands into a temp. file and let octave evaluate this. The line by line procedure resulted from the wish that the commands should be contained in the history and displayed at he command line as if they were executed one by one in the command line.

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sat 09 May 2020 09:16:07 AM UTC, comment #9: 

Instead of operating on the marked text line-by-line, would it be possible to execute the whole marked text (and drop it completely in to the helper function) and let the parser do its job? Only thing necessary might be to escape \n and \r where necessary.

It's probably only necessary to split and modify the text if it contains "keyboard" (or "dbstop"?). Or would it even be save to always surround the commands by the "__db_next_breakpoint_quiet__" calls?

But it is quite possible that I don't see the complete picture yet.

Markus Mützel <mmuetzel>
Project Member
Sat 09 May 2020 08:16:34 AM UTC, comment #8: 

@Markus: You are right. I have tested with an older version without the fix, sorry. I will check how the latest changeset affects cases like the one in comment #5. I am currently not seeing a way to handle comments in continued lines without re-parsing the complete code.

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Sat 09 May 2020 07:52:09 AM UTC, comment #7: 

It doesn't work for me with hg id 01358faf49c9.

Also comments after a continuation marker cause the same parse error.
In the following example, only marking the two lines for "b" (and executing with F9) works:

a = {1,
     2};

b = {1, ...
     2};

c = {1, ... This is a comment
     2};

For "a" and "c", I get something similar to:

>> error: parse error

  syntax error

>>> octave_OhVpCp (1, '     2};', '     2};');

Markus Mützel <mmuetzel>
Project Member
Sat 09 May 2020 07:04:06 AM UTC, comment #6: 

@Markus: Your example works for me. Maybe it was fixed "accidentically" by the fix for the original issue?

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Fri 08 May 2020 08:12:41 PM UTC, comment #5: 

Probably a related issue: Multiline commands that don't use the continuation marker fail with a very similar error when executed with F9.
E.g. the following code works in a script file but not when executed with F9:

a = {1,
     2};

Markus Mützel <mmuetzel>
Project Member
Fri 08 May 2020 07:02:22 PM UTC, comment #4: 
Torsten Lilge <ttl>
Project MemberIn charge of this item.
Fri 08 May 2020 04:51:58 AM UTC, comment #3: 

I have changed the category, since this is an issue of the GUI's way to evaluate the selected editor text.

Torsten Lilge <ttl>
Project MemberIn charge of this item.
Thu 07 May 2020 03:58:30 PM UTC, comment #2: 

sorry for being incomplete.

If the following command is executed from the gui editor (e.g. with F9)

x = ...
1

one gets

>> error: parse error


  syntax error

>>> octave_eczKyS (1, '1', '1');

                               ^

This worked in older (<6) versions. I am currently using

GNU Octave Version: 6.0.1 (hg id: ce7a5b60e102)

Muhali <muhali>
Thu 07 May 2020 03:39:43 PM UTC, comment #1: 

Do you mean that it fails on the command line or in some other context?

Exactly what do you mean by "not parsed"?

Is there an error message?

If there is no error message, can you show exactly what happens?

What hg ID are you using?

John W. Eaton <jwe>
Project Administrator
Thu 07 May 2020 01:45:29 PM UTC, original submission:  

similar to bug #58198, continuation markers are not parsed, as in

x = ...
1

Muhali <muhali>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by ttl (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by muhali (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-05-09 mmuetzel Attached File- => Added history_grouped_error.png, #49051
    2020-05-08 mmuetzel Operating SystemGNU/Linux => Any
    2020-05-08 ttl StatusConfirmed => Ready For Test
    2020-05-08 ttl CategoryInterpreter => GUI
        Assigned toNone => ttl
    2020-05-07 jwe StatusWorks For Me => Confirmed
        Summary'...' not being parsed correctly => '...' continuation marker causes parse error when executed with "run selection" in GUI editor
    2020-05-07 jwe StatusNone => Works For Me

    Back to the top


    Powered by Savane 3.5