Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
jsx-newline
: support for checking newline at the start and end of children#3678
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 ourterms of service andprivacy 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?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
checkStartEnd: { | ||
default: false, | ||
type: 'boolean', | ||
}, |
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.
@ljharb this makes sense but I wonder if we shouldn't also include option for banning start/end newlines while forcing empty lines between children 🤔
So this might actually become an enum offorce
|prevent
rather than boolean. What do you think, what should I go with?
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.
Or we might go with:
undefined
andfalse
= do nothingtrue
= behaves same as between children (as perprevent
)force
= forces an empty lineprevent
= prevents empty line
And it will still takeallowMultilines
into account
codecovbot commentedJan 11, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## master #3678 +/- ##==========================================+ Coverage 97.65% 97.74% +0.08%========================================== Files 132 132 Lines 9397 9405 +8 Branches 3445 3446 +1 ==========================================+ Hits 9177 9193 +16+ Misses 220 212 -8 ☔ View full report in Codecov by Sentry. |
I like the idea of effectively deprecating the boolean config, and instead allowing an enum of |
Kinda feel like deprecating (while still supporting) old config, but making new config be: {between:'force'|'force-one'|'prevent'|'ignore'// between childrenadjecent:'force'|'force-one'|'prevent'|'ignore'// before first child and after last child}
Obviously if both are Those properties might probably be called something else (better?). |
"between" seems weird for a parent/child relationship. Are you just trying to think of possible combinations, or do you have use cases? A linter saying "adjacent" usually means lexically, not like "siblings" in terms of the AST graph. |
burtek commentedJan 12, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
So I want to distinguish all four options I saw in different projects: <parent><childA/><childB/></parent> <parent><childA/><childB/></parent> <parent><childA/><childB/></parent> <parent><childA/><childB/></parent> Easiest way would be allow configuring the rule for newlines between siblings separately from newlines before first and after last sibling. And the choice between |
ljharb commentedJan 12, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
OK, let's aim for those 4 options, with an eye for future extensibility. That means two keys, one for siblings and one for directly inside of a parent, each of which a "must have a newline" value, and a "must not have a newline" value. I suppose we could also have "ignore", too. What about (allowMultilines would apply to all permutations) |
Sounds good |
380e32c
to51d342b
Compare
Fixes#3663