Fixed issue concerning different line ending on Windows

[spainer]

Fixed issue concerning different line ending on Windows when using Synctex.

[izuzak]

Hey @spainer :wave: -- thanks for opening a pull request! Just to make sure I understand, what problem exactly does this solve for you? If you click on a line in the PDF right now -- what unexpected behavior do you see? I don't have a Windows VM handy to test things myself, so just asking. :v:

Also going to cc @autc04 who added this recently.

[autc04]

Ah yes, now that you mention it, yes that is necessary. The symptom should be SyncTeX just not working because the extra \r that is part of Windows line endings is taken to be part of the tex file name otherwise.

I recommend merging.

[izuzak]

:ok_hand: Thanks for taking a look, @autc04 :bow:

[izuzak]

And of course - thanks @spainer for the fix! :sweat_smile:

[spainer]

You are welcome. And yes, the symptom was that just nothing happened on clicking inside the PDF.

[spainer]

@izuzak I am sorry, but there was a mistake in the fix. I don't know why it worked yesterday but after updating the package today, it did not work any more. The fix yesterday only replaced the first occurence of CR+LF, not all occurences. I pushed a new commit to the branch. Is it possible to reuse this PR to merge again or do I need to open a new PR for that?

@autc04 Just to clarify: The problem was with the regular expression, not with a wrong file name. The regular expression is not valid if there is a CR at the end of the line and there m is null. Perhaps one should think about adding \s* at the beginning and the end of the line to get more robust or just call trim() before evalutating the regular expression.

[izuzak]

Thanks for noticing that, @spainer.

I pushed a new commit to the branch. Is it possible to reuse this PR to merge again or do I need to open a new PR for that?

I could cherry-pick that commit, but I think it's better if we do this through a new PR. Before we do that, I'm wondering if you could clarify one thing:

The regular expression is not valid if there is a CR at the end of the line and there m is null.

If there is a CR at the end of that line, wouldn't that still be matched by the (.*) in the regex? A . matches anything, right? So line.match /^([a-zA-Z]*):(.*)$/ should match any line beginning with letters and a colon afterwards because (.*) will match anything until the end of the line. I definitely might be wrong, I normally am when regexes are involved. :smile:

Still, the line number would not be correct because it would contain a CR at the end, which I think is what @autc04 was referring to.

[izuzak]

If there is a CR at the end of that line, wouldn't that still be matched by the (.*) in the regex? A . matches anything, right?

Nope, looks like I was wrong -- the match will fail if there's a CR there, the dot doesn't match it. Just tried it out.

@spainer I'll cherry pick your commit and push a new release.

[izuzak]

Just published v0.34.0 -- give it a try, @spainer and let me know if you notice any other problems. Thanks again :zap:

[spainer]

You were faster than me, I was just in the middle of a message about the dot :wink: Thanks for cherry picking.

[spainer]

v0.34.0 now works correctly.