bugGNU Octave - Bugs: bug #43972, "strread" command with...

 
 

bug #43972: "strread" command with "commentstyle","matlab" does not work

Submitter:  None
Submitted:  Sat 10 Jan 2015 10:16:53 PM UTC
   
 
Category:  None Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Edwin de Koning Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 3.8.1
Operating System:  * Any Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 11 Feb 2015 09:06:34 PM UTC, comment #10: 
Philip Nienhuis <philipnienhuis>
Group Member
Wed 11 Feb 2015 08:28:04 PM UTC, comment #9: 

@Massimiliano: thanks very much for this fix & sorry for a bit of delay.
Fixed strread works fine in 3.9.1+.

I'll polish it up a bit (Octave coding style, more concise message) and push later tonight or tomorrow (depending on when my build finishes)

Philip Nienhuis <philipnienhuis>
Group Member
Fri 30 Jan 2015 08:13:55 AM UTC, comment #8: 

For the time being, I have just updated the previous patch for 4.1.0+ adding some tests.

No need to add me, though: my name is already in the list of contributors and my email in the list of translators.

(file #32946)

Massimiliano Fasi <mfasi>
Sun 25 Jan 2015 06:02:33 PM UTC, comment #7: 

@Massimiliano:
Before I forget: is your name already in the list of contributors?
If not, would you like to be included?  (only your name; your email address will appear a bit hidden in the repo (in the changeset), i.e. not very accessible)

Philip Nienhuis <philipnienhuis>
Group Member
Sun 25 Jan 2015 05:58:24 PM UTC, comment #6: 

In my own builds I always copy the trio strread,m/textread.m/textscan.m over from the default branch, because they contain several bug fixes and improvements that I need myself (and that I made myself).
But for the official repo that is of course no option.

To repeat myself, I have no idea why 3.9.0+ and 3.8.2 behave differently, as the comment-processing part of strread.m hasn't been touched for a long time. The only difference could be changes in external functions called in that part of the code (i.e. functions that have changed in 4.1.0+).
Anyway, this is just my curiosity and not very important as your bug fix will probably go to default (= future 4.2.x).

As to the test, something like:

%% bug #43972
%!test
%! assert (strread("Hello World! % this is comment","%s", -1, ...
%!                  "commentstyle","matlab"), ...
%!         {"Hello", "World!"});

might be a start (straight from my head, not tested!)

Philip Nienhuis <philipnienhuis>
Group Member
Sun 25 Jan 2015 08:15:46 AM UTC, comment #5: 

Thank you for having checked the patch. In fact, it was just for the 4.1.0+ release, the bug is not the same on 3.9.0+:

  • 4.1.0+: last char is displayed


  • 3.9.0+: comments are ignored


I didn't add any test but I think that could be easily done. Also, I could try to understand the behaviour of the 3.9.0+ version of strread and to fix it. Could a backport of strread from 4.1.0+ to 3.9.0+ be the solution?

Massimiliano Fasi <mfasi>
Sat 24 Jan 2015 10:58:52 PM UTC, comment #4: 

OK the patch works fine on 4.1.0+, but not on my 3.9.0+. Hmm...

@Massimiliano:
Can you add one or more tests to the patch, please? the OP's example problem is already one.

BTW why the first hunk of the patch? I see no difference...

Philip Nienhuis <philipnienhuis>
Group Member
Sat 24 Jan 2015 08:39:10 PM UTC, comment #3: 

Thanks, I'll have a try on 3.8.2 / 3.9.0+ as well.
The part of the code in question hasn't changed since -I think- 3.6.0 so it should work on stable too.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 18 Jan 2015 09:30:23 AM UTC, comment #2: 

I attach a patch that should fix the problem on 4.1.0+.

(file #32861)

Massimiliano Fasi <mfasi>
Sat 10 Jan 2015 11:30:54 PM UTC, comment #1: 

Confirmed on 3.9.0+.

On 4.1.0+ I get for the second example:

strread("Hello World! % this is comment","%s", -1, "commentstyle","matlab")
ans =
{
  [1,1] = Hello
  [2,1] = World!
  [3,1] = t
}


Hmmm.
That said, I may have copied strread.m from 4.1.0+ to 3.9.0+ (I have a combined 3.9.0+/4.1.0+ setup on my Windows box), in which case there may be additional complications only valid for my box.
But even then this bug is confirmed.

OS -> Any

Philip Nienhuis <philipnienhuis>
Group Member
Sat 10 Jan 2015 10:16:53 PM UTC, original submission:  

The strread command with a "commentstyle" as either "shell", "c++" or "matlab" does not work. If the "commentstyle" is "c"then it does work.

Example:

>> strread("Hello World! /* this is comment*/","%s", -1, "commentstyle","c")

ans =
{
  [1,1] = Hello
  [2,1] = World!
}
This works as expected.



>> strread("Hello World! % this is comment","%s", -1, "commentstyle","matlab")

ans =
{
  [1,1] = Hello
  [2,1] = World!
  [3,1] = %
  [4,1] = this
  [5,1] = is
  [6,1] = comment
}
This is wrong. The comment after the % sign is still present in the answer.
It also goes wrong for the other options "shell" or "c++" where the comment has to be removed until the End-Of-Line character.



Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #32861:  bugfix_43972.patch added by mfasi (1KiB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mfasi (Updated 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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-02-11 philipnienhuis StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2015-01-30 mfasi Attached File- Added bugfix_43972_31012015.patch, #32946
    2015-01-24 philipnienhuis StatusConfirmed Patch Submitted
    2015-01-18 mfasi Attached File- Added bugfix_43972.patch, #32861
    2015-01-10 philipnienhuis StatusNone Confirmed
        Operating SystemGNU/Linux Any

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code