-
Notifications
You must be signed in to change notification settings - Fork 75
Limiting max levels to 5 #656
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
Conversation
Signed-off-by: Steve Springett <steve@springett.us>
Thanks for the contribution. This seems like something which should be opt-in, and also should generate a warning since you'll often want manual remediation. I've pushed up commits doing both (+ a test). If this still works for you I'll land. |
@bakkot opt-in sounds reasonable as does displaying a warning. |
So we're just flattening the numbering and not the actual structure of the |
I don't see why it would be? From what I understand the ISO standards only specify a limit on the numbers themselves, not on the presentation of clauses. Changing how we number clauses doesn't seem to me like it ought to interact with CSS or anything else. |
I really don't like the auto-collapsing in this PR. I would strongly advise against it. The computer doesn't know, when collapsing a 6-time-nested clause, whether it's more appropriate to collapse the leaf into its parent or any one of the other 5 parent nodes along the way into their parent instead. That's a decision that the author should make. If it were up to me, I would make this a warning only, which could be caused to fail the build using |
The clause numbering should always coincide with the nesting of |
This PR does not do any auto-collapsing. (I originally thought it did but I was misreading.) It only touches the numbering.
That's a valid preference but unless you intend to use this feature I would prefer to defer to the preferences of the people who are actually going to use this. The warning is still emitted and enforceable with |
I prefer not to include features that I think are both broken in implementation and footguns, but as you point out, I have no plans to use the feature myself, so I'm not going to get in your way. |
Version 21.4.0 is out including the new |
Closes #655