Skip to content
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

Git status representation in the explorer viewlet implemented #8462

Closed

Conversation

kisstkondoros
Copy link
Contributor

Resolves #5866

image

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @egamma and @bpasero to be potential reviewers

@bpasero
Copy link
Member

bpasero commented Jun 29, 2016

Unfortunately I cannot accept this PR as such because it introduces a dependency from files land to git land. Logically files land should not be aware of git land but rather we should have extension points that allow us to modify the colors and possibly icons in the explorer from the outside. This would maybe also allow extensions to do this.

@bpasero bpasero added this to the Backlog milestone Jun 29, 2016
@bpasero bpasero self-assigned this Jun 29, 2016
@kisstkondoros
Copy link
Contributor Author

@bpasero If I got you right then we could do the following:
The FileRenderers get's a registry whose items can be processed in renderContents method.
This way there is no thight coupling, and this registry could be used maybe even from extensions.

the interface in use could be e.g.

interface FileRendererDecorator {
    decorateItem(tree: ITree, stat: FileStat, explorerItemLabel: HTMLElement) : void
}

perhaps this could even be used to support icons in the explorer.

@bpasero
Copy link
Member

bpasero commented Jun 29, 2016

Yes, we need a registry where other components can participate in decorating explorer items. I actually think this should be generic enough for any tree we have and not just the explorer. Trees can then opt in to decide if they want to participate in decorations or not and the explorer could be the first one of those.

Have not yet thought about how the API (both internal and possibly external could look like). Maybe we open a discussion issue first?

@kisstkondoros
Copy link
Contributor Author

@bpasero Good idea, please open a discussion issue when you have some time 👍

@kisstkondoros
Copy link
Contributor Author

@bpasero I've updated the implementation according to what we have discussed, please have a look

@bpasero
Copy link
Member

bpasero commented Jun 30, 2016

Thanks, since it is currently endgame, expect no reply until July milestone 👍

A first example is also included: gitStatusTreeDecorator

Resolves microsoft#5866
Related microsoft#211
@kisstkondoros
Copy link
Contributor Author

Some more related issues: #1394, #211, #4837, #1086, #178

@bpasero
Copy link
Member

bpasero commented Jul 6, 2016

Adding @joaomoreno because this PR touches a lot in tree and git land.

@Tyriar Tyriar mentioned this pull request Jul 8, 2016
@johanneslamers
Copy link

Oh yes please!! As soon as possible!!

@joaomoreno
Copy link
Member

joaomoreno commented Jul 11, 2016

I think there are a few shortcomings with the current approach:

  • Augmenting the tree's API is unnecessary. The tree provides a structure around HTML elements. The contents of each tree row, after render is called, are not the tree's responsibility no more. So, adding a decorator API at that layer is wrong.
  • There are also layer breakers such as git knowing about OpenEditor and FileStat, or worse, knowing about how these are rendered: row.element.querySelector('.open-editor .name').

Given that I am tempted to close this issue. It needs proper planning, architecture and most of all: time.

@kisstkondoros kisstkondoros deleted the gitinexplorerview branch August 3, 2018 21:49
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
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.

Show git status for files in explorer
6 participants