Mon 07 Nov 2016 06:26:56 PM UTC, comment #14:
TTBOMK strread.m hasn't changed since 4.2.0 and 4.3+ were branched off from 4.1.0+ so it should be safe to backport it to stable.
But there is probably no hurry, it could be in a 4.2.1 bug fix release as well.
|
Wed 02 Nov 2016 08:55:17 PM UTC, comment #13:
You could if you like. I think the fact that it took forever for someone to notice this means it isn't a requirement to fix it before the next release. I guess if you 'hg graft' it from default to stable and there are a bunch of merges to figure out then it can be done.
|
Wed 02 Nov 2016 06:33:30 PM UTC, comment #12:
Would it be useful to backport the fix to stable? as it is a mere bug fix.
|
Sat 29 Oct 2016 08:55:58 AM UTC, comment #11:
Rik:
In hindsight I think I could have been more clear, sorry.
The word "FIXME" shouldn't have been there in the first place.
Thanks for reviewing & pushing
|
Fri 28 Oct 2016 10:50:50 PM UTC, comment #10:
Oh, I see. I don't think we need to support decade-old operating systems. I know we dropped support for some of the arcane compilers like Cray, and probably even DEC. I reverted my cset, although I still removed the FIXME since we aren't going to do it. See http://hg.savannah.gnu.org/hgweb/octave/rev/90c3839825a3.
|
Fri 28 Oct 2016 08:51:51 PM UTC, comment #9:
@Rik:
The FIXME was about old Macintosh OS, not the current OS-X.
Nowadays OS-X also has LF ("\n") as EOL char.
That's why I left the FIXME in there - do we really need to cater for an operating system from over a decade ago?
|
Fri 28 Oct 2016 05:47:26 PM UTC, comment #8:
I added to the commit message what the changes in the code were and then checked it in on the development branch (http://hg.savannah.gnu.org/hgweb/octave/rev/644dcdfa0348).
There was a FIXME in the code about handling comments on Mac platforms which I fixed in this cset (http://hg.savannah.gnu.org/hgweb/octave/rev/b100f76b991d).
Closing report.
|
Fri 28 Oct 2016 01:23:51 PM UTC, comment #7:
@Mike comment #2:
Matlab r2012a has the same issue as Octave; comments need a delimiter ("word boundary") around them. So yes, Octave's textscan is Matlab-compatible here, but it's a little disappointing.
Octave's previous textscan.m was better in this respect; it could also distinguish cuddling literals.
This was discussed with Lachlan in patch #8783 in a.o., comment #99 and comment #5.
|
Fri 28 Oct 2016 10:15:42 AM UTC, comment #6:
Please try the following cset
If you think it works fine it could be backported to stable as well.
(file #38830)
|
Fri 28 Oct 2016 09:16:12 AM UTC, comment #5:
(My work appointment (on my regular off-day) was just canceled, so I got some time :-) )
The issue is twofold:
- as outlined below, strread.m needs the "endofline" value to assess comment endings.
- strread.m as it stands always wants to eat the comment ending character (default "endofline" value), but in "open" comment styles like "shell" that is non-existent. Consequently in the example in the comments below it eats the newline following the comment but that is actually a delimiter;
As "open" comment styles generally end with a newline (what else?) I think my suggestion in my previous comment would work fine. I already have a fix but need to add a few tests.
|
Thu 27 Oct 2016 08:44:46 PM UTC, comment #4:
Yeah it's quite a while since I fiddled with strread.m / textread.m / textscan.m:-)
Some observations:
------------------
The plan was (is?) to make strread.m a front end for textscan(); it used to be the other way round until ~a year ago.
strread.m in Matlab is not deprecated but it is recommended to rather use textscan. (In the Matlab docs the stanza that strread "will be removed in a future version of Matlab" has disappeared; so I conclude that deprecation sensu stricto is not planned.)
I made a start with converting strread.m a year ago after Lachlan created the binary txtscan() but ran in several issues. As textscan is a better alternative, also recommended by Matlab, I wonder how much energy we should put into keeping strread.m up-to-date anyway.
AFAIK TMW doesn't do much about it either. For example, about a year ago I entered a bug report about the default emptyvalue (then documented to be NaN, but Matlab's strread returned 0 (zero)); TMW simply changed the docs rather than fix strread.
As to the comment processing:
-----------------------------
IIRC that part has been provided by Rik. He probably doesn't remember :-)
In Matlab strread.m has no option "endofline". I've put it into the Octave version at the time as strread.m is/was the backend for textread and textscan that do have that option. It was much more convenient to handle endofline processing in strread.m. Assessment of the "endofline" value is/was done in textread.m and textscan.m
I think we should not use the "endofline" value but silently assume a LF or CRLF during comment processing; to be overridden when "endofline" has been specified by the user. The motive is that (IIRC) strread.m's default endofline value is empty for a reason (that I do not have at hand right now).
Unlike strread, textscan returns all output from interpreting string/file contents into one cell array so I think the output in comment #2 is correct. The word boundary thing can be checked in Matlab.
Until after next November I have very little time/priority to fix strread.m.
|
Thu 27 Oct 2016 04:44:20 PM UTC, comment #3:
(Trying to fix the property changes I stepped on.)
Looking at strread.m it is correctly ignoring text between the comment character and the EOL character. The problem is that there is no default EOL character in the function unless the "endofline" property is given.
It seems a shame that textscan and strread have a lot of overlap, but suffer from different bug symptoms.
|
Thu 27 Oct 2016 04:32:56 PM UTC, comment #2:
Confirmed, with a couple of workarounds.
The strread function has been deprecated in Matlab, if you use textscan instead you can get the result that you are looking for, with a slight change
I observe that textscan only recognizes commentstyle when it comes after a word boundary, that may be a separate bug.
For strread, you can use the undocumented "endofline" property to correctly get it to do what you want
Except that this is still not quite what you were looking for. In Octave's strread, it has simply deleted all text between the comment character and the newline, and the result forms a single word.
Can you or someone test these examples in Matlab to see what changes need to be made if any?
|
Thu 27 Oct 2016 04:31:12 PM UTC, comment #1:
Confirmed. The problem is general to any of the commentstyle options. For example,
also returns just "hello".
The strread function is in an m-file in scripts/io so it is easy enough to debug. The problem is that the End-of-Line character in the variable eol_char is empty. Starting at around line 325 in strread.m
I'm adding Philip to the CC list since he understands this function better than anyone else.
|
Thu 27 Oct 2016 07:15:04 AM UTC, original submission:
I would expect that
would return
but it returns "hello" instead.
|