trim whitespace from comments before parsing #2287
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
thanks for the contribution @dominicbarnes , could you pull from master to fix the failed tests? |
|
Sorry about the delay, I'll do that right now. |
61a01ad to
dd5f10f
Compare
|
@jamengual looks like tests pass! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I ran into a minor UX issue when copy-pasting a command for Atlantis. It looks like Github added an extra line before and after my comment, causing my comment to be silently ignored, after which I thought the apply was just hanging.
Based on the code comments around this, it looks like the intent was to at least allow trailing whitespace for this reason, so I reasoned it might be ok to allow leading whitespace as well. This change just trims whitespace from the left and right of the comment before parsing it, in an attempt to make things a little more flexible. I've added test cases for both leading whitespace and whitespace on both sides, in addition to the trailing whitespace test cases from before.
It's possible that blindling trimming whitespace could be problematic, so I'm open-minded to tightening this up if desired. This is my first contribution, so just learning the ropes here.