Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Couple of fixes #188

Merged
merged 4 commits into from
Mar 8, 2018
Merged

Couple of fixes #188

merged 4 commits into from
Mar 8, 2018

Conversation

karthiknadig
Copy link
Member

Fixes #165
Fixes #116

ptvsd/wrapper.py Outdated
single_underscore.append(var)
else:
variables.append(var)
variables = variables + single_underscore + double_underscore + dunder
Copy link
Contributor

@int19h int19h Mar 8, 2018

Choose a reason for hiding this comment

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

While you're at it, can you also do sorted() on variables? Then this will completely fix microsoft/PTVS#2865

Copy link
Contributor

Choose a reason for hiding this comment

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

Or rather sorted() on every component individually, since we still want to preserve the category order.

@karthiknadig
Copy link
Member Author

@int19h I used sort instead of sorted since I am using a list and sort works in-place. Will that work?

ptvsd/wrapper.py Outdated
double_underscore.sort(key=get_sort_key)
dunder.sort(key=get_sort_key)

variables = variables + single_underscore + double_underscore + dunder
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd move all of this into a separate class to track and sort the variables.
If tomorrow we need to sort private variables or member variables, we need to modify this method again, when logic of sorting the list should be outside.

Copy link
Contributor

Choose a reason for hiding this comment

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

This stuff or sorting and tracking variables should be in its own class, something that can be easily unit tested in isolation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'll need unit tests for this sorting rule.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will move this to a class before merging.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

ptvsd/wrapper.py Outdated
single_underscore = [] # variables that begin with underscores
double_underscore = [] # variables that begin with two underscores
dunder = [] # variables that begin and end with double underscores
variables = VariablesSorter()
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@karthiknadig karthiknadig merged commit daaa1a4 into microsoft:master Mar 8, 2018
@karthiknadig karthiknadig deleted the preview2 branch May 1, 2018 22:13
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.

3 participants