microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.56k stars 12.43k forks source link

Better outlining spans for prototype method assignments #31516

Closed DanielRosenwasser closed 5 years ago

DanielRosenwasser commented 5 years ago

Keywords: outline outlining spans navtree nav tree folding prototype

From: https://twitter.com/code/status/1130893425090256897

x.prototype.initialize = function() {};
y.prototype.initialize = function() {};
z.prototype.initialize = function() {};

Expected:

Outlining spans for navtree are something like

x.prototype.initialize
y.prototype.initialize
z.prototype.initialize

or potentially

image

Actual:

image

xiata commented 5 years ago

My mental expectations of it since they are similar concepts. image

dragomirtitian commented 5 years ago

FYI: I have started working on this.

@DanielRosenwasser I think the idea of doing x.initialize is actually less appealing than nesting the attached elements (as @xiata suggests) from a UI perspective.

I think I can get your version (x.initalize) done faster as, from my digging, the changes required don't seem to be very big.

The nesting approach is a bit more complex, and we should consider several issues:

  1. Do we change the type (ie the icon) of the item to class if there are any prototype assignments to a function ? I think yes.
  2. Do we add a separate node for the original function as a constructor ? I think yes, in order not to mix any nested constructor functions with member functions.
  3. What do we do about members directly attached to the function (not the prototype). Currently these are presented as standalone elements that are not in any way related to the function they are getting attached to: image

I would like to nest these under the function as well as long as I am doing work in this area.

DanielRosenwasser commented 5 years ago

@dragomirtitian looks like you're on the right track.

So to answer questions

  1. Yes
  2. I guess I'd have to see what that looks like - it'd be confusing to have two Foos unless one was nested under the other.
  3. Is there a way to mark one of those methods as static? I think they should be nested, but displayed the same as a static method in a class.
dragomirtitian commented 5 years ago

@DanielRosenwasser Yup, making good progress 😊

  1. What I meant was creating an extra node under the function. With two cases: a. When there are prototype assignments, we consider this to be a class, so we add a constructor node with function locals nested under this: image b. When there are no prototype assignments, we leave this as a function, maybe add a locals node for definitions actually inside the function, although this new locals node would be inconsistent with other functions. image Although I am not sure mixing the locals with the attached members would be a good experience either: image

  2. There does not currently seem to be a visual distinction between static and non-static class members so they would not be visually different. Might be worth considering adding a static member icon in the future: image

DanielRosenwasser commented 5 years ago

Any thoughts @mjbvz @amcasey ?

amcasey commented 5 years ago

@DanielRosenwasser I don't believe VS has a specific outline view. Do you happen to know if the VS navbars are backed by the same command(s)?

dragomirtitian commented 5 years ago

@amcasey Yes the navbar is backed by the same code as the outline view (the changes I already made show up in both the outline view and the nav bar).

Just to be clear what you mean by "I don't believe VS has a specific outline view", you mean that the same views are used for both navbars and outline, right ? Not that VS Code does not have an outline view by default, because I disabled all my extensions and I still have the outline view shown in the pictures above.

amcasey commented 5 years ago

@dragomirtitian my questions were about Visual Studio, rather than VS Code. I'd be pleased to learn that (regular) VS has an Outline view, but I'm not aware of one.

dragomirtitian commented 5 years ago

@amcasey Wops, misread that, my bad.

Taxman972 commented 5 years ago

Is there any chance to implement this in Visual Studio Code ?

jessetrinity commented 5 years ago

Is there any chance to implement this in Visual Studio Code ?

VSCode already uses the navtree so you should see the corresponding changes whenever the shipped version updates appropriately. Alternatively, you can experiment with the latest version of TypeScript yourself.

DanielRosenwasser commented 5 years ago

There's actually an extension to automatically use the nightlies now: https://marketplace.visualstudio.com/items?itemName=ms-vscode.vscode-typescript-next

Taxman972 commented 5 years ago

@jessetrinity @DanielRosenwasser, Thank you very much.