-
Notifications
You must be signed in to change notification settings - Fork 75
Enhance the Standalone Indentation test #93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Does this change actually solve anything? I mean, I don't think you can pass the new version of the test without also passing the old version. If it does solve something, I would prefer the new version to coexist alongside the old version. |
The indentation should only be applied at the start of a line.
673d40c
to
ecabc91
Compare
@jgonggrijp Hey, sorry for the delay I'd missed the ping. From what I can remember, we had an implementation bug in the rust implementation of this and the main tests for the library were leveraging these specs so it felt worthwhile to adapt these to also help potential future implementers. The library was passing the old test but without this change it was passing this new one. I've rebased to add it as a new test case instead of modifying the other example. |
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.
Thank you for separating this from the existing test. With some slight changes (see comments), I think we can accept this. Would you be able to update the JSON as well?
@spullara or @bobthecow, please take a quick look if you can.
specs/partials.yml
Outdated
@@ -106,6 +106,27 @@ tests: | |||
| | |||
/ | |||
|
|||
- name: Standalone Indentation |
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.
Please add something to this name
to distinguish it from the previous test.
specs/partials.yml
Outdated
@@ -106,6 +106,27 @@ tests: | |||
| | |||
/ | |||
|
|||
- name: Standalone Indentation | |||
desc: Indentation should only be applied after each newline in a partial. |
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.
This description is not strictly true, because the indentation should also be applied to the first line, before any newline.
@jgonggrijp updated and re-built the json files |
The indentation should only be applied at the start of a line.