Skip to content
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

Add indentation to hidden counter text #604

Merged
merged 8 commits into from
Aug 27, 2024

Conversation

gibson042
Copy link
Contributor

This results in better copied text, e.g.

b. Set length to length   1.
c. Let weight be GetWeight of node.
d. If length = 10**9 or weight > 2**(length   1), then
   i. NOTE: This is an out-of-bounds state.
   ii. If ignoreErrors is not true, then
       1. Throw a RangeError exception.
   iii. Set hadError to true.

rather than

b. Set length to length   1.
c. Let weight be GetWeight of node.
d. If length = 10**9 or weight > 2**(length   1), then
i. NOTE: This is an out-of-bounds state.
ii. If ignoreErrors is not true, then
1. Throw a RangeError exception.
iii. Set hadError to true.

@bakkot
Copy link
Contributor

bakkot commented Aug 26, 2024

Thanks! I don't like that the whitespace is included even if just copying a single line, though, as one does fairly often when writing an implementation which closely adheres to the spec text. I'm a bit hesitant to mess with the copy event, but it's probably appropriate here.

@gibson042
Copy link
Contributor Author

OK, that makes sense. Done.

@bakkot
Copy link
Contributor

bakkot commented Aug 27, 2024

I think that logic is broken in Chrome and Safari, which give slightly different selection ranges when triple-clicking.

I can try to fix it later if you haven't gotten to it first.

@gibson042
Copy link
Contributor Author

@bakkot Updated; please retest and let me know about any issues you encounter.

@bakkot
Copy link
Contributor

bakkot commented Aug 27, 2024

Looks like it works for steps with substeps, but not steps without. Specifically, triple-clicking on the "This is an out-of-bounds state" line in the docs ends up with the following tree structure in domRoot, which doesn't trigger the conditions you're checking for:

Screenshot 2024-08-27 at 12 10 26 PM

@gibson042
Copy link
Contributor Author

Thanks for the detail. Fixed.

@gibson042 gibson042 force-pushed the 2024-08-hidden-counter-indentation branch from b77b1ae to 9e07a32 Compare August 27, 2024 21:56
Copy link
Contributor

@bakkot bakkot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bakkot bakkot merged commit 248cd38 into tc39:main Aug 27, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants