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

Show squigglies on collapsed regions containing errors or warnings (#120989) #190682

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

gjsjohnmurray
Copy link
Contributor

This PR closes #120989

junk

@gjsjohnmurray
Copy link
Contributor Author

@aeschli is folding your domain?

@gjsjohnmurray
Copy link
Contributor Author

Tweaked to reduce dominance of the squigglies:

image

@aeschli
Copy link
Contributor

aeschli commented Aug 25, 2023

Thanks @gjsjohnmurray, Having the squigglies on the line number seems strange to me. What about annotating the... decorator? Could be squiggles or part of the decorator text.

@hediet @daviddossett Any ideas?

@daviddossett
Copy link
Contributor

Cool idea! I tend to agree. Either a decoration the chevron icon (i.e. chevron-dot like, bell-dot or similar) or a squiggle on the ... makes sense to me.

@gjsjohnmurray
Copy link
Contributor Author

Thanks for the feedback. Drawbacks I see to putting the decoration under the ... are that it won't always be visible if the first line of the folded section is long enough to require horizontal scrolling, and that ... may not be wide enough for the pattern differentiation between error squigglies and warning squigglies to be discernible (important for people unable to differentiate between the two colours).

I'm willing to give it a try, but I assume I've missed the cutoff for 1.82.

@gjsjohnmurray
Copy link
Contributor Author

I had a go at simulating the appearance of a 'chevron-dot' by using 'debug-coverage'

image

I think it's too cryptic. Nor would it differentiate between folds containing errors and those containing only warnings, unless we come up with two decorations.

I tried unsuccessfully to put the squiggles under the ... at the end of the folded line, or under the chevron-right at the start. I don't understand enough about how the editor CSS works.

I experimented with additional text after the ..., for instance ... (contains errors) but felt that looked ugly. Plus, there's another proposal to append the count of folded lines there.

@gjsjohnmurray
Copy link
Contributor Author

Any more thoughts about this?

@gjsjohnmurray
Copy link
Contributor Author

@hbons is UX your area? If so I would appreciate your input on this.

@daviddossett
Copy link
Contributor

Apologies, I've been on paternity leave until recently and this fell off my radar. Looking at it with fresh eyes, I wonder if we could just put the squiggles on whatever characters are visible when a region is collapsed. Once expanded, the squiggles are removed from the parent and show on the specific lines with errors.

I worry about changing the collapse iconography with similar logic. Once opened, we would suddenly change the icon with the error indicator to some nested level, and so on as more regions are expanded. That seems easier to reason about with squiggles, less so with icons.

@gjsjohnmurray
Copy link
Contributor Author

@daviddossett congratulations! Re icons, I don't like that idea either. But I think squiggling the whole of the text in the folded line would be confusing, implying that this line itself was entirely in error. Plus, if part of the line genuinely did deserve squiggles we would lose sight of that until we expanded it.

I still favour squiggles in the gutter.

@gjsjohnmurray
Copy link
Contributor Author

It will soon be the first anniversary of this PR of mine
@daviddossett - any further thoughts about refining / accepting it?

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.

[folding] Decorate fold indicators with error color/iconography
3 participants