bugGNU Octave - Bugs: bug #40894, unrecognised block comments in dos...

 
 

bug #40894: unrecognised block comments in dos format saved files

Submitter:  John Donoghue <lostbard>
Submitted:  Sat 14 Dec 2013 02:52:05 AM UTC
   
 
Category:  Interpreter Severity:  4 - Important
Priority:  7 - High Item Group:  Incorrect Result
Status:  Fixed Assigned to:  jwe
Originator Name:  John D Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 15 Dec 2013 06:14:59 PM UTC, comment #8: 
John Donoghue <lostbard>
Group Member
Sun 15 Dec 2013 04:08:31 PM UTC, comment #7: 

Oh, that looks much better.  Please check it in on the stable branch.

Thanks!

John W. Eaton <jwe>
Group administrator
Sun 15 Dec 2013 01:05:49 AM UTC, comment #6: 

I played around with the parser for a while, and think I fixed the issue with a simple fix (attached)



(file #29922)

John Donoghue <lostbard>
Group Member
Sat 14 Dec 2013 01:35:48 PM UTC, comment #5: 

I also tried the 1st patch which fixes the issue with a start %{.
It also works for all the examples below except:


%b=1
%{
test
%}
c=1


John Donoghue <lostbard>
Group Member
Sat 14 Dec 2013 12:41:42 PM UTC, comment #4: 

The 2nd patch fixes it for starting the file with a block comment.

However it appears that anything that is not valid statements
(ie: new lines, line comment) prior to the block comment also doesn't work:


%a=1
%{
test
%}
c=1


and


<newlinehere>
%{
test
%}
c=1


and


%a=1
<newlinehere>
%{
test
%}
c=1


and


%a=1

%{
test
%}

Attempts run run 'test'

John Donoghue <lostbard>
Group Member
Sat 14 Dec 2013 05:33:49 AM UTC, comment #3: 

Thanks for noticing that it is only failing at the beginning of the file.  So I think I see what I was missing before.  I'm guessing that the problem is that the pattern


<INPUT_FILE_START>{ANY_INCLUDING_NL}


that is matched at the beginning of a script or function file means that the next match can't be anchored by the beginning of line pattern ^.  Thankfully, the only other patterns that use the line anchor are associated with exclusive start states so they won't be matched at the beginning of a file.

However, now I'm not sure YY_AT_BOL does what I thought, so the previous patch is probably not correct.

Maybe something like the second attached patch will work?


(file #29917)

John W. Eaton <jwe>
Group administrator
Sat 14 Dec 2013 04:36:47 AM UTC, comment #2: 

We currently have two patterns to match comments in the lexer.  They appear in the following order:


^{S}*{CCHAR}\{{S}*{NL}  --> block comment start
{S}*{CCHAR}.*{NL}       --> line comment start


The flex docs say that when two patterns match and the text that they match is the same length, then the action that is used is supposed to be the one corresponding to the pattern that appears first in the lex input file.  So unless I'm missing something, I think this may be a bug in flex.  But it looks like we can also work around the problem with the attached patch.  Please test it and let me know if it solves the problem for you.

I suppose that we should apply this change to the stable branch since block comments won't work for files with CRLF line endings and that seems like a fairly serious and obvious problem that will probably affect a lot of people.  Does anyone object to making the change on the stable branch?



(file #29916)

John W. Eaton <jwe>
Group administrator
Sat 14 Dec 2013 03:34:29 AM UTC, comment #1: 

A valid command before the block comment, and the block comment works as expected in both DOS and UNIX.

ie:

a=1
%{
test
%}
c=1


works as expected

John Donoghue <lostbard>
Group Member
Sat 14 Dec 2013 02:52:05 AM UTC, original submission:  

In 3.8.0-rc1 .m file created:


%{
test
%}


If saved in DOS format (\r\n) the block comment isn't seen and 'test' will be run.
On saving the file in UNIX format, the block comment works as expected.

It does the same in Windows and Linux.

John Donoghue <lostbard>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #29922:  fix_dos_cr.patch added by lostbard (935B - application/octet-stream)
file #29917:  diffs.txt added by jwe (2KiB - text/plain)
file #29916:  diffs.txt added by jwe (4KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by lostbard (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-12-15 lostbard StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2013-12-15 lostbard Attached File- Added fix_dos_cr.patch, #29922
    2013-12-14 jwe Attached File- Added diffs.txt, #29917
    2013-12-14 jwe Severity3 - Normal 4 - Important
        Priority5 - Normal 7 - High
        StatusNone Patch Submitted
        Assigned toNone jwe
    2013-12-14 jwe Attached File- Added diffs.txt, #29916

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code