Skip to content

Conversation

lionel-
Copy link
Contributor

@lionel- lionel- commented Jul 5, 2025

For #8330
Extension side of posit-dev/ark#859

Can be merged separately.

Copy link

github-actions bot commented Jul 5, 2025

E2E Tests 🚀
This PR will run tests tagged with: @:critical

readme  valid tags

Copy link
Contributor

@juliasilge juliasilge left a comment

Choose a reason for hiding this comment

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

Thank you so much! 🙌

},
"positron.r.symbols.includeAssignmentsInBlocks": {
"type": "boolean",
"default": false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"default": false,
"default": true,

I am not a fan of this change, I'm afraid. 😬

I did some experimentation with realistic projects and the old treatment is preferable for everything I tried.

BEFORE:

Screenshot 2025-07-23 at 7 57 13 PM

AFTER:

Screenshot 2025-07-23 at 7 57 17 PM

Situations where one would not want to include assignments within blocks in the symbols have got to be pretty rare, so let's make this new behavior opt in instead of opt out.

Copy link
Contributor Author

@lionel- lionel- Jul 24, 2025

Choose a reason for hiding this comment

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

I'd like to discuss this further because I think the point at which the outline becomes too noisy is likely very common.

In packages with a few short functions, as in the screenshots above, I agree it's not that bad. But
even in this best case scenario, is it really helpful to see local variables in the outline? Local variables are more likely to have vague and similar names, and thus more likely to distract rather than inform.

More importantly the main purpose of the outline is to show the structure of the document. The outline is especially important to get oriented in large files with many functions and methods. In these large files, I think local variables clearly distract from the general outline as you can see in the testthat examples brought up by Jenny and Davis who also feel that the current behaviour yields a noisy outline: posit-dev/ark#856 (comment). This will also be the case with e.g. large R6 classes with many methods, and other similar long file / many functions scenarios. Even in your example, I find it much easier to get the general picture of the document in the After screenshot.

It's also worth noting that Python does not show local variables either. Having a consistent experience across languages seems like a plus in a multilingual IDE.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here are some more examples from ellmer.

Numerous local variables in print.ellmer_prompt(), some with vague/nondescript names:

Screenshot 2025-07-25 at 12 48 18

Long file where the general outline is obscured by local variables:

Screen.Recording.2025-07-25.at.12.49.12.mov

Same file with the new setting turned off. It's much easier to see the file structure, i.e. the outline:

Screen.Recording.2025-07-25.at.12.49.47.mov

Copy link
Contributor

Choose a reason for hiding this comment

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

We chatted and we are going with the default that is more like RStudio and Jedi, and less like Pylance.

juliasilge
juliasilge previously approved these changes Jul 28, 2025
Branched from #8412

For #4886
Extension side of posit-dev/ark#866

Can be merged separately.
@lionel- lionel- changed the base branch from main to bump/ark July 29, 2025 06:33
@lionel- lionel- merged commit ba26e86 into bump/ark Jul 29, 2025
11 checks passed
@lionel- lionel- deleted the feature/symbols-top-level branch July 29, 2025 06:34
@github-actions github-actions bot locked and limited conversation to collaborators Jul 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants