Closed tecknojock closed 7 years ago
Hmmm... I imagine I meant /s and not /w but I think all eclipses posts might not have had trailing white space after all
On Sun, Jul 16, 2017, 20:09 Andrew Rodgers-Schatz notifications@github.com wrote:
@xiagu commented on this pull request.
In lib/jekyll/rp_logs/rp_logline.rb https://github.com/xiagu/jekyll-rp_logs/pull/75#discussion_r127620952:
@@ -145,7 +145,13 @@ def mergeable_with?(next_line) end
def merge!(next_line)
- @contents += "#{space_between_lines}#{next_line.contents}"
- if /...\w$/.match(@contents) && /^\w.../.match(next_line.contents)
But those don't catch trailing spaces, they would match, like, Lorem ipsum...abc and def...dolor sit amet and merge it into Lorem ipsum dolor sit amet. Is there an example file where there was an issue that this solved? I'd like to make a test out of it if so.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/xiagu/jekyll-rp_logs/pull/75#discussion_r127620952, or mute the thread https://github.com/notifications/unsubscribe-auth/AEkTmaX1oZeCwhlXKqP0QVkSDERDe8FXks5sOqYpgaJpZM4KcYHZ .
The previous one just did not handle eclipses at. I think the chances of it having affected other logs minimal since am eclipses without a space at the start and end would be a fairly rare occurrence
On Sun, Jul 16, 2017, 20:24 Jonathan Davis tecknojock@gmail.com wrote:
Hmmm... I imagine I meant /s and not /w but I think all eclipses posts might not have had trailing white space after all
On Sun, Jul 16, 2017, 20:09 Andrew Rodgers-Schatz < notifications@github.com> wrote:
@xiagu commented on this pull request.
In lib/jekyll/rp_logs/rp_logline.rb https://github.com/xiagu/jekyll-rp_logs/pull/75#discussion_r127620952:
@@ -145,7 +145,13 @@ def mergeable_with?(next_line) end
def merge!(next_line)
- @contents += "#{space_between_lines}#{next_line.contents}"
- if /...\w$/.match(@contents) && /^\w.../.match(next_line.contents)
But those don't catch trailing spaces, they would match, like, Lorem ipsum...abc and def...dolor sit amet and merge it into Lorem ipsum dolor sit amet. Is there an example file where there was an issue that this solved? I'd like to make a test out of it if so.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/xiagu/jekyll-rp_logs/pull/75#discussion_r127620952, or mute the thread https://github.com/notifications/unsubscribe-auth/AEkTmaX1oZeCwhlXKqP0QVkSDERDe8FXks5sOqYpgaJpZM4KcYHZ .
https://github.com/xiagu/jekyll-rp_logs/issues/6