Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trailing Carriage Returns #8

Closed
delta42 opened this issue Apr 27, 2018 · 3 comments
Closed

Trailing Carriage Returns #8

delta42 opened this issue Apr 27, 2018 · 3 comments
Labels

Comments

@delta42
Copy link

delta42 commented Apr 27, 2018

I find that if I only have one carriage return after my last SRT string, this last subtitle gets ignored by the parser. Therefore I need to have exactly 2 carriage returns after the last subtitle.

If, on the other hand, I have more carriage returns, then the last subtitle will get repeated in the vector of SubtitleItems. This is not really a big deal since the timestamps are all the same for those repeated items, but it might be related to the first issue I mention here.

Just thought I'd mention this possible improvement.
Otherwise nice portable parser.
Thanks.

@delta42
Copy link
Author

delta42 commented May 2, 2018

I'm thinking that this may be something that works slightly different on Linux and Windows?
I'm suggesting this because on Windows I never witnessed infile.eof() being true in SubRipParser::parse.

The bottom of this function now looks like this for me:

		else if (turn)
		{
			turn = 0;
			_subtitles.push_back(new SubtitleItem(subNo,start,end,completeLine));
			completeLine = timeLine = "";
		}

		if(infile.eof())    //insert last remaining subtitle
		{
			turn = 0;
			_subtitles.push_back(new SubtitleItem(subNo,start,end,completeLine));
		}
	}

	if (turn)
	{
		_subtitles.push_back(new SubtitleItem(subNo, start, end, completeLine));
	}

This works for me on Windows for the cases where I have any number of carriage returns after the last line of text in the SRT file, including 0.

But I don't have the possibility to test on Linux so I'm not proposing it as a fix, just a possible solution to consider if anybody else is witnessing the same symptom under Windows.

@saurabhshri
Copy link
Owner

Oh, this definitely sounds like a bug! Sorry for the super-late response.

Thanks for the report!

@saurabhshri
Copy link
Owner

Same bug : #7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants