-
Notifications
You must be signed in to change notification settings - Fork 81
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
alter getCurrentBlockRange request #755
Conversation
loc0 = loc + a.args[1].fullspan + a.args[2].fullspan | ||
if loc <= offset <= loc + a.span | ||
if CSTParser.defines_module(a) # Within module at the top-level, lets see if we can select on of the block arguments | ||
if loc <= offset <= loc + a.args[1].span # Within `module` keyword, so return entire expression |
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.
This would be really helpful, while we still want julia-vscode/julia-vscode#1330 to be solved for great module experience.
end | ||
loc += b.fullspan | ||
end | ||
else | ||
p1, p2, p3 = loc, loc + a.span, loc + a.fullspan | ||
elseif loc + a.args[1].fullspan + a.args[2].fullspan + a.args[3].fullspan < offset < loc + a.args[1].fullspan + a.args[2].fullspan + a.args[3].fullspan + a.args[4].span # Within `end` of the module, so return entire expression |
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.
so this branch tries to return the entire module code at when we query request in end
keyword, right ?
how about "extending" this condition so that this gets hit when the position is "just after" the end
keyword ?
The test sample in my mind will be right
module A
a rand(Bool)
end|
where |
represents the request position.
Currently this case never hits this branch and so just returns empty range.
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.
That case works fine for me.
Will fix julia-vscode/julia-vscode#1281 as well |
end | ||
end | ||
loc += a.fullspan | ||
end | ||
end | ||
return Position(get_position_at(doc, p1)...), Position(get_position_at(doc, p2)...), Position(get_position_at(doc, p3)...) | ||
return Position(0, 0), Position(0, 0), tdpp.position |
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.
Could we try to get the next block here as well instead of returning the current cursor position?
We probably need to merge some version of this before a release, right? |
Yes. Let's merge this as-is -- the inline comments aren't a big deal and can imho be addressed in a follow-up PR since this fixes a pretty annoying bug. |
fixes #716 ?
Changes the behaviour to ignore trailing white space when considering which block to retrieve. By default returns an empty selection at (0,0):(0,0). @pfitzseb , is this what was wanted?