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

 
 

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

Submitted by:  None
Submitted on:  Sat 10 Jan 2015 10:16:53 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: None
Originator Name: Edwin de KoningOriginator Email: -unavailable-
Open/Closed: ClosedRelease: 3.8.1
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 Jump to the original submission)

Wed 11 Feb 2015 09:06:34 PM UTC, comment #10:

pushed:
http://hg.savannah.gnu.org/hgweb/octave/rev/4aa17069a007

Closing report

Philip Nienhuis <philipnienhuis>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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>
Project 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:

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>
Project 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 File(s):
   
   
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
  • -unavailable- added by mfasi (Updated the item)
  •  

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

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 11 Feb 2015 09:06:34 PM UTCphilipnienhuisStatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Fri 30 Jan 2015 08:13:55 AM UTCmfasiAttached File-=>Added bugfix_43972_31012015.patch, #32946
    Sat 24 Jan 2015 08:39:10 PM UTCphilipnienhuisStatusConfirmed=>Patch Submitted
    Sun 18 Jan 2015 09:30:23 AM UTCmfasiAttached File-=>Added bugfix_43972.patch, #32861
    Sat 10 Jan 2015 11:30:54 PM UTCphilipnienhuisStatusNone=>Confirmed
      Operating SystemGNU/Linux=>Any

    Back to the top


    Powered by Savane 3.1-cleanup1