Skip to content

Collect sections in calls and emit them as document symbols #867

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

Open
wants to merge 1 commit into
base: feature/optional-section-workspace-symbols
Choose a base branch
from

Conversation

lionel-
Copy link
Contributor

@lionel- lionel- commented Jul 7, 2025

Branched from #866.
Addresses posit-dev/positron#8402.

You can now add sections in function calls too, between arguments.

The implementation is shared with the section logic for {} blocks. This works a little differently than in RStudio: The sections are nested in the call and can't affect the section hierarchy outside the call. If you have:

## level 2 ----

call(
  # level 1 ----
)

call({
  # level 1 ----
})

The "level 1" sections in the block and call do not close the top-level "level 2" section. This difference is necessary because the LSP outline is more powerful and includes syntactic elements like function definitions and assigned variables in the outline. Allowing sections to close from inside a nested element would make things complicated and I'd argue respecting the nesting of the code makes sense from a user pespective too.

QA Notes

Sections in calls:

# level 1 ----

list(
  ## foo ----
  1,
  2, ## bar ----
  3,
  4
  ## baz ----
)

## level 2 ----

list(
  # foo ----
  1,
  2, # bar ----
  3,
  4
  # baz ----
)

now work the same way as sections in blocks:

# level 1 ----

list({
  ## foo ----
  1
  2 ## bar ----
  3
  4
  ## baz ----
})

## level 2 ----

list({
  # foo ----
  1
  2 # bar ----
  3
  4
  # baz ----
})
Screen.Recording.2025-07-07.at.15.31.07.mov

Copy link
Contributor

@DavisVaughan DavisVaughan left a comment

Choose a reason for hiding this comment

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

Blocking so we can at least discuss my comment above together before merging

@@ -177,12 +177,12 @@ fn collect_symbols(
) -> anyhow::Result<()> {
Copy link
Contributor

Choose a reason for hiding this comment

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

So I guess you view a function call as a kind of "reset" of the nesting level right?

I know you mentioned an example like this in your description, but explicitly compare positron and rstudio here:

Image Image

At first glance I thought RStudio made more sense here, as it seems to only rely on the number of # to determine how the Outline works.

But then I did this, and I think I like how Positron works here more

Image Image

It's like you're saying:

"The call() implicitly starts an H2 ## nesting level under level 2, but when you write sections in call(), you can "start over" with a single #"

I don't know how to precisely describe that feature. It kind of feels right, but it's also fairly weird to see # level 1-a and ## this show up at the same level in the outline, because they have a differing number of leading #. It makes it kind of hard to predict?

I also wonder if differing from RStudio too much here is going to make the outline annoying when coming from RStudio with a big {targets} script...

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I take it back that I like Positron more in the second example.

I think having # level 1-a show up under # level 2 is too confusing.

I really think the number of # should drive the nesting level, as that is very easily predictable (possibly it is the only thing driving the nesting level?)

i.e. I think letting the number of # drive the nesting level would lean into the principle of least surprise here.

It also seems like it would align with RStudio more.

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