Put dunder global variables in their own scope

[karthiknadig]

Ideally the following should go into their own group.

• builtin dunder variables
• data
• functions, classes etc

[fabioz]

Note: I'm taking a look at this.

[fabioz]

Also related: #763

[fabioz]

Also related: #118

[fabioz]

@karthiknadig @int19h I'm still working this issue (along with the other related issues I commented above) -- I'm in the process of updating test cases now and checking for corner cases.

I separated entries in 4 groups (dunder variables, protected variables, function variables, class variables) -- anything that doesn't enter one of those groups appears directly below the variable (note that a variable goes into the group it matches first there even if it could appear at more than one group).

So, I'd like to gather comments on whether you think that's ok or if you think something should be changed in that organization...

The current result is in the screenshot below.

[int19h]

It looks great. We might want to fiddle with what exactly goes into which category if there's conflicts, but that can be a separate enhancement.

@brettcannon Is there some better name than "dunder variables" for that category?

[int19h]

One other thing we'll need is a boolean property in debug configuration to enable/disable grouping, something like "groupVariables". This will allow vscode-python to gradually roll it out via the experiment framework, and for users to explicitly opt out afterwards.

[brettcannon]

The ones in the example are technically global variables. They are also all standard module attributes. So it really depends on what you're trying to filter out: things that just happen to be named with a leading and trailing __ or specifically module attributes that exist on all/most modules?

[int19h]

I think technically we want to filter out the "magic" variables. It doesn't have anything to do with whether it's globals or not - as you can see on the screenshot, self also has that category.

[fabioz]

@brettcannon

For me it's a way to show variables that are seldomly used (and this includes both heuristics based on the name and variable type) without cluttering the UI too much.

As a bit of history, until now the debugger was always filtering out all of those (so, they'd only be accessible by having the user evaluate them on the cases he actually needed them) -- you can see the actual logic in https://github.com/microsoft/debugpy/blob/0dd33dd250a884bfa2b4f5025b15d2a29575646c/src/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_resolver.py#L187 -- with the proposed changes, the user would be able to access those through a group that can be expanded.

@int19h
When that setting is turned on, those variables would disappear (as was the case until now) or would they show along all other variables?

[int19h]

I was thinking of the setting that covers this whole functionality (i.e. also protected, function etc). But we might want to think about something more fine-grained than that - e.g. some users might not like separating functions into a different category, and some might not like protected, but will still want dunder. So, maybe something like:

"groupVariables": ["function", "class", "dunder"]

It's unfortunate that we have to do this on the debugger side. Really, what it should be doing is just reporting variables tagged with categories, and the client should then show them in groups (or not) according to their configuration. Users shouldn't have to edit launch.json for this, because it's not a session-specific preference, but user-specific. Which means that the extension will have to surface it in user settings, and then inject it into debug configuration, much like we do with "pythonPath".

So, basically like DAP scopes, but applicable on every level of the hierarchy. This needs DAP and client support though.

[int19h]

Thinking about this more - since we'd want to also accommodate the existing behavior of hiding dunder entirely, how about:

"variablePresentation": {
    "function": "show",
    "dunder": "hide",
    "class": "group",
    ...
}

[brettcannon]

So basically you're trying to group Python-defined or Python-created variables? Maybe something along those names if you're going to include self?

[int19h]

The "dunder" group does not include self. What I was saying is that the screenshot shows a "dunder" group under self, so it's not just for predefined module variables. It's really any variable with __ around, but the intended purpose of this is to show "magic" ones separate from regular ones.

[brettcannon]

Technically they are special methods, informally called magic methods, and are typically pronounced "dunder ".

[brettcannon]

So take your pick? 😉 Or ask Luciana or someone who might not have as much exposure to the concept to see which name makes the most sense. (My guess is not "dunder" since you won't know that pronunciation without hearing someone say it and it typically isn't written down.)

[karthiknadig]

/cc @luabud

[fabioz]

I'll rename dunder variables to special variables as it seems better for me -- and I like the idea of configuring it with:

"variablePresentation": {
    "function": "show",
    "dunder": "hide",
    "class": "group",
    ...
}

If someone has a different opinion, please let me know ;)

[luabud]

I really like Pavel's suggestion and I also prefer special over dunder. My only question is regarding the difference between "show" and "group". Would it be that those who have "show" in it would be displayed all together without a parent?

[int19h]

Yep, that's the idea. Basically, the current behavior, just for that particular category.

Maybe it's not the best pair of terms to denote the difference, since "group" also shows the item, just in a different way. I don't know what the good concise term for "show without grouping" would be - "inline", maybe? We need native English speakers :)

[karthiknadig]

/cc @rchiodo We are using this mechanism as a way to reduce what we show in the variables window.

[fabioz]

Ok, I've provided a pull request where the options are group, inline, hide.

The default is:

        "variablePresentation": {
            "special": "group",
            "function": "group",
            "class_": "group",
            "protected": "inline",
        },

It's also possible to specify an entry with all: <some option> which sets the default value for all entries -- that way it's possible to do something as:

        "variablePresentation": {
            "all": "hide",
            "protected": "inline",
        },

I plan on merging later today.

[adnan333bd]

Thinking about this more - since we'd want to also accommodate the existing behavior of hiding dunder entirely, how about:

"variablePresentation": {
    "function": "show",
    "dunder": "hide",
    "class": "group",
    ...
}

where this configuration goes to, launch.json ?

[int19h]

@adnan333bd Yep, it should be in your launch.json if using VSCode.

Note that the category names changed slightly in the later comments - you're using "dunder", but it's "special" now.