-
Notifications
You must be signed in to change notification settings - Fork 298
Add indents to every line in the snippet. #2394
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2394 +/- ##
==========================================
+ Coverage 38.2% 38.23% +0.03%
==========================================
Files 300 300
Lines 12518 12530 +12
Branches 1649 1650 +1
==========================================
+ Hits 4782 4791 +9
- Misses 7481 7484 +3
Partials 255 255
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent @CrossR 👍 thats been a real annoyance
@CrossR I think if the snippets are subdivided by line then adding an indent should be fine worst case a line might wrap because of indentation but I dont think anything would blow up, also from the indent lines plugin Im pretty sure detect indent handles empty lines fine as well |
Just gave this an actual test (vs just a visual test of the lines) and realised the way I was doing it wasn't updating the placeholders, so tabbing through left you in the old positions. I'm pushing up a change for that now, that fixes it on my end. |
Nice catch @CrossR ! It would be worth having a test case to document this behavior - we have some examples here:
(I think it'd be pretty similar to that test, except we'd want to have some whitespace in the buffer - all the tests at the moment just insert a snippet to an empty buffer). |
Unit tests added now, is it worth adding some for the tabbing through placeholders? Technically I didn't actually change any of that, since the stuff I've added is all done before that. |
|
||
const [firstLine, secondLine, thirdLine] = await mockBuffer.getLines(0, 3) | ||
|
||
assert.strictEqual(firstLine, "\tfor {") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks for adding these tests! 💯
This is great! Just glad we have the indentation behavior covered. I think if we find bugs with the placeholder behavior we could add some additional ones to cover it, at that time. |
Fixes #2242.
Adds the correct indent level to every line in a snippet.
Is there any edge cases I'm missing where this could fall apart?