bugGNU Octave - Bugs: bug #64903, Command window may not accept...

 
 

bug #64903: Command window may not accept multiple line comments

Submitter:  Liang Tang <lt1234>
Submitted:  Thu 16 Nov 2023 05:34:29 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  lt1234 Open/Closed:  * Closed
Release:  * stable Operating System:  * Any
Fixed Release:  9.1.0 Planned Release:  9.1.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 24 Nov 2023 10:13:55 PM UTC, comment #11: 

Closing as fixed.

Arun Giridhar <arungiridhar>
Group Member
Sun 19 Nov 2023 08:25:48 PM UTC, comment #10: 

Thanks.  I registered a new issue.

Liang Tang <lt1234>
Sun 19 Nov 2023 08:04:14 PM UTC, comment #9: 

Does that behavior with pause show up afresh with this patch or was it there before the patch? If it was there before, please open a new bug report about it.

Arun Giridhar <arungiridhar>
Group Member
Sun 19 Nov 2023 07:22:18 PM UTC, comment #8: 

I ran into another case and would like you to comment.  "pause"  seems to be problematic.  Editor I have is notepad++. 

Please cut/paste these below into command window.  These two groups are the same except, for the latter, a carriage return is entered right after the pause. 

When I pasted the lines, the first pause does not work at all. 0 and 1 were displayed automatically without any feedback from me. After all done, I then used arrow up key back to the first line, pause works under this condition.

For the 2nd pause, at the end of the line, below, the first character of the line is always ignored/missing. 

display(0); pause, display(1)
display(0); pause,
display(1)


>>
>> display(0); pause, display(1)

0
1

>> isplay(0); pause,

error: 'isplay' undefined near line 1, column 1

>> display(1)

Liang Tang <lt1234>
Sat 18 Nov 2023 11:19:44 PM UTC, comment #7: 

Hi Arun, looks good. Thanks.

Liang Tang <lt1234>
Fri 17 Nov 2023 06:01:01 PM UTC, comment #6: 

I pushed it here since it passed the tests I tried with it: https://hg.savannah.gnu.org/hgweb/octave/rev/bdbc67b5f92e

See if it works as intended for you when you get the overnight build.

Marking as ready for test.

Arun Giridhar <arungiridhar>
Group Member
Fri 17 Nov 2023 04:06:04 PM UTC, comment #5: 

Hi Arun, Thanks for the info.   For the non-m-file change, I can only download the nightly build and test. 

I don't have the understanding of the updates you made.  I think the same issue also applies to while and perhaps other similar statement/command that I don't know.

Liang Tang <lt1234>
Fri 17 Nov 2023 03:58:13 PM UTC, comment #4: 

I've got it. With the attached patch, this is the behavior:

Code to paste:

for i = 1:5
i
#{
  foo
  #{
    bar
  #}
  baz
#}
-i
end


Output:


octave:1> for i = 1:5
> i
> #{
>   foo
>   #{
>     bar
>   #}
>   baz
> #}
> -i
> end
i = 1
ans = -1
i = 2
ans = -2
i = 3
ans = -3
i = 4
ans = -4
i = 5
ans = -5
octave:2>


Patch, also attached:


diff -r b5e433b52bc3 libinterp/parse-tree/lex.ll
--- a/libinterp/parse-tree/lex.ll       Mon Nov 13 16:58:52 2023 +0100
+++ b/libinterp/parse-tree/lex.ll       Fri Nov 17 10:50:29 2023 -0500
@@ -762,16 +762,17 @@ ANY_INCLUDING_NL (.|{NL})

     curr_lexer->m_block_comment_nesting_level--;

-    int status = -1;
-
     if (curr_lexer->m_block_comment_nesting_level == 0)
       {
-        status = -2;
-
         curr_lexer->pop_start_state ();
+
+        if (curr_lexer->pending_token_count () > 0)
+          HANDLE_EOB_OR_EOF (-1);
+        else
+          HANDLE_EOB_OR_EOF (-2);
       }
-
-    HANDLE_EOB_OR_EOF (status);
+    else
+      HANDLE_EOB_OR_EOF (-1);
   }

 %{


Please test this out. If it works as intended for you, I'll push it with bug number etc.

(file #55336)

Arun Giridhar <arungiridhar>
Group Member
Thu 16 Nov 2023 09:31:20 PM UTC, comment #3: 

Looking through hg log for lex.ll, the most relevant changeset is this: https://hg.savannah.gnu.org/hgweb/octave/rev/d938c4d22200

Related: bug #58198, which was marked as a regression blocker for Octave 6. Marking this current bug as a regression since it worked before Octave 6.

This attempt at a patch is on the right path but not fully correct. I tried to adapt the single-line comment parsing code a little below this part of lex.ll, as seen in the above changeset. With the patch, it doesn't end immediately after the block comment ends, but it doesn't end after the for loop ends either! Needs more work...


diff -r b5e433b52bc3 libinterp/parse-tree/lex.ll
--- a/libinterp/parse-tree/lex.ll       Mon Nov 13 16:58:52 2023 +0100
+++ b/libinterp/parse-tree/lex.ll       Thu Nov 16 16:18:37 2023 -0500
@@ -762,16 +762,21 @@ ANY_INCLUDING_NL (.|{NL})

     curr_lexer->m_block_comment_nesting_level--;

-    int status = -1;
-
-    if (curr_lexer->m_block_comment_nesting_level == 0)
+    if (curr_lexer->pending_token_count () > 0)
+      {
+        HANDLE_EOB_OR_EOF (-1);
+      }
+    else if (curr_lexer->m_block_comment_nesting_level == 0)
       {
-        status = -2;
-
         curr_lexer->pop_start_state ();
+        HANDLE_EOB_OR_EOF (-2);
       }
-
-    HANDLE_EOB_OR_EOF (status);
+    else
+      {
+        curr_lexer->pop_start_state ();
+        curr_lexer->xunput (yytext[0]);
+      }
+
   }

 %{


Arun Giridhar <arungiridhar>
Group Member
Thu 16 Nov 2023 08:18:04 PM UTC, comment #2: 

I agreed that it worked in 5.2.  Thanks.

Liang Tang <lt1234>
Thu 16 Nov 2023 06:18:50 PM UTC, comment #1: 

I can confirm that this used to work in Octave 5.2.0.
It no longer works in Octave 6.4.0.

This might start working again once Octave switches to the new GUI terminal widget (which will use a push parser instead of a pull parser).
See: https://wiki.octave.org/wiki/index.php?title=GUI_terminal_widget (which might need an update...)

Markus Mützel <mmuetzel>
Group administrator
Thu 16 Nov 2023 05:34:29 PM UTC, original submission:  

Octave used to accept multiple line comments in command window as matlab always does.   The example below shows the error when the multiple line comments are grouped with other statement/command.  Thanks. 


for ii=1:1
%{       
%}
end

>> for ii=1:1

%{
%}

>> end

error: parse error:

  syntax error

>>> end

      ^

>>

Liang Tang <lt1234>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #55336:  lex.patch added by arungiridhar (671B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by arungiridhar (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by lt1234 (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-11-24 arungiridhar StatusReady For Test Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 9.1.0
    2023-11-17 arungiridhar StatusPatch Submitted Ready For Test
        Planned ReleaseNone 9.1.0
    2023-11-17 arungiridhar SummaryCommand window may not accept mutliple line comments Command window may not accept multiple line comments
    2023-11-17 arungiridhar Attached File- Added lex.patch, #55336
        StatusConfirmed Patch Submitted
    2023-11-16 arungiridhar Item GroupMatlab Compatibility Regression
        Release8.2.0 stable
        Operating SystemMicrosoft Windows Any
    2023-11-16 mmuetzel StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-df36.
    Corresponding source code