parsers/base: make sure content always ends with an empty line

[xaf]

For some whois output where the content does not end with an empty new line, the whois call was returning an Unexpected token: % (c) 2019 Canadian Internet Registration Authority, (http://www.cira.ca/) error, for instance.

This fixes that by forcing into existence that ending new line.

Note that this is going to be followed by another PR to support the last CIRA whois output, which is currently failing.

[joallard]

Looking at the build, looks like this is Ruby 2.5+ only because of String#delete_suffix.

[xaf]

Hah! Trying with gsub to fix that then :)

[xaf]

Seems there are issues with other registrars now... Unrelated to what I changed

[joallard]

@xaf I guess that as long as it doesn't introduce new test breaks, it's mergeable.

(Interesting fixup technique by the way, I figure you show your fixups but offer squashing that right before the merge?)

[joallard]

As a matter of readability, it's not immidiately clear what we're doing here and why we're doing it.

[xaf]

What change would you suggest ?
Would that be acceptable ?

Suggested change

	@content_for_scanner ||= "#{content.to_s.gsub(/\r\n/, "\n").gsub(/\n$/, '')}\n"
	@content_for_scanner ||= begin
	content_str = content.to_s
	content_str.gsub!(/\r\n/, "\n")
	# We need a new line at the end, which exists in some cases, and does
	# not in others, we will thus remove the ending new line character if it
	# exists (or do nothing if it does not), then return the content appended
	# with a new line character
	content_str.gsub!(/\n$/, '')
	"#{content_str}\n"
	end

[joallard]

So I understand that it's better for the scanner to ingest whole lines (including linebreak) instead of just the string on it, thus rendering the line empty but still present, right?

[xaf]

Yes

[xaf]

@xaf I guess that as long as it doesn't introduce new test breaks, it's mergeable.

(Interesting fixup technique by the way, I figure you show your fixups but offer squashing that right before the merge?)

Sorry for the delay in getting back to you for this @joallard (wow, and what a long delay...)
I actually have a git command to fast create the fixup. Locally I generally do a git rebase -i --autosquash once everything is ready (and git push -f in this case). Adding new fixup commits instead of force-pushing during development allow reviewers on github to see what they've not seen yet instead of restarting the review from scratch. Another possibility for the squash is that the person merging the commit in GitHub uses the Squash and merge option, which does the autosquash automatically (that person would then have to remove in the description all the separate commits though, and keep only the main one, since all the fixup! are not really interesting to see in the git log)