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

selectParentSyntax does not select whole file #1436

Closed
jrvidal opened this issue Sep 6, 2024 · 4 comments
Closed

selectParentSyntax does not select whole file #1436

jrvidal opened this issue Sep 6, 2024 · 4 comments

Comments

@jrvidal
Copy link

jrvidal commented Sep 6, 2024

Describe the issue

It seems like selectParentSyntax() is unable to extend the selection all the way up to the syntax tree root (e.g. the whole Script in a JS file). I'm not sure this is intended, but I do find it surprising.

For instance, in the demo editor in https://codemirror.net, if a click on console and hit Ctrl+i repeatedly, it gets stuck in the function block.

Seems like the culprit is here, but I don't know what the node.parent?.parent check is supposed to prevent.

Browser and platform

No response

Reproduction link

No response

@marijnh
Copy link
Member

marijnh commented Sep 6, 2024

This is intentional behavior. You have ctrl-a to select the entire file. Treating it as a syntactic unit doesn't really add anything meaningful.

@marijnh marijnh closed this as completed Sep 6, 2024
@jrvidal
Copy link
Author

jrvidal commented Sep 6, 2024

That's fair, I suppose I can use the returned boolean as a hint to extend the selection manually myself.

However, I'll note that it doesn't just prevent the selection to cover the whole file. It also blocks top-level items from being selected e.g. in the chain ... -> Block -> FunctionDeclaration -> Script, I'm stuck at Block. And that's not something I can work around on my own as easily.

EDIT: oh, wait, selectParentSyntax() always returns true. Is that something that would make sense to change? Not sure if false is supposed to mean "unable to run" or, instead, "no changes to be made".

marijnh added a commit to codemirror/commands that referenced this issue Sep 7, 2024
FIX: Fix an issue causing `selectParentSyntax` to not select syntax that
is a direct child of the top node.

Closes codemirror/dev#1436
marijnh added a commit to codemirror/commands that referenced this issue Sep 7, 2024
FIX: Make `selectParentSyntax` return false when it doesn't change the selection.

Issue codemirror/dev#1436
@marijnh
Copy link
Member

marijnh commented Sep 7, 2024

Oh I see what you mean. That isn't intentional. Attached patches should improve it.

@jrvidal
Copy link
Author

jrvidal commented Sep 7, 2024

Thanks for the prompt response, those fixes will come in handy.

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

No branches or pull requests

2 participants