weppos / whois-parser

An intelligent — pure Ruby — WHOIS parser.
https://whoisrb.org
MIT License
99 stars 102 forks source link

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

Open XaF opened 4 years ago

XaF commented 4 years ago

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 commented 4 years ago

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

XaF commented 4 years ago

Hah! Trying with gsub to fix that then :)

XaF commented 4 years ago

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

joallard commented 4 years ago

@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?)

XaF commented 4 years ago

@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)