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

SCM history: Show details on hover (fix #200378) #202262

Closed
wants to merge 16 commits into from

Conversation

gjsjohnmurray
Copy link
Contributor

This PR fixes #200378

@gjsjohnmurray
Copy link
Contributor Author

/assign @lszomoru

@gjsjohnmurray
Copy link
Contributor Author

image

@lszomoru
Copy link
Member

@gjsjohnmurray, thank you very much for the pull request. I was thinking that for each incoming/outgoing entry we would want to show a rich hover similar to the timeline entry. If you do not have bandwidth to update the PR, I am happy to take it from here.

@gjsjohnmurray
Copy link
Contributor Author

I have pushed a change to make this display a Markdown-capable tooltip. I think the proposed API will probably need a tooltip? : MarkdownString property adding to SourceControlHistoryItem and wiring through to ISCMHistoryItem so the Git extension can pass a Git-appropriate tooltip.

@gjsjohnmurray gjsjohnmurray marked this pull request as draft January 15, 2024 23:52
@lszomoru lszomoru added this to the February 2024 milestone Jan 16, 2024
@gjsjohnmurray gjsjohnmurray marked this pull request as ready for review January 17, 2024 18:22
@gjsjohnmurray
Copy link
Contributor Author

I have added vscode.SourceControlHistoryItem.tooltip to the proposed API and used it from the Git extension.

@gjsjohnmurray
Copy link
Contributor Author

image

@gjsjohnmurray gjsjohnmurray changed the title SCM history: Show full label on hover (fix #200378) SCM history: Show details on hover (fix #200378) Jan 18, 2024
@gjsjohnmurray
Copy link
Contributor Author

Please can this be reviewed (and merged 🤞) soon. I really like the new incoming/outgoing feature but it's tedious to have to widen my side bar to see the full commit messages and the authors.

@gjsjohnmurray
Copy link
Contributor Author

@lszomoru any chance this can be merged in time for February?

@lszomoru
Copy link
Member

Sorry for not getting back to you on this until now. The "Timeline" proposed API is "UI driven" so it makes sense for it to have a property with which the extension describes how the hover should look. "The History Provider" API is model driven so I do not think that we should be making any changes to the existing API. We should just use the existing information (author, message, etc) to render a rich hover.

@lszomoru lszomoru modified the milestones: February 2024, March 2024 Feb 16, 2024
@gjsjohnmurray
Copy link
Contributor Author

@lszomoru I think a history item "rich" hover will be impoverished if all it does is display untruncated the label and description properties of the item (which is what my original commit did, using a native tooltip). Please help me understand why you oppose allowing the extension that provides the item to also provide a markdown tooltip conveying whatever additional information the extension deems relevant.

@gjsjohnmurray
Copy link
Contributor Author

@lszomoru can I do anything to progress this?

@lszomoru lszomoru modified the milestones: March 2024, April 2024 Mar 26, 2024
@gjsjohnmurray
Copy link
Contributor Author

@lszomoru I guess this has been made obsolete by your #209373

@lszomoru
Copy link
Member

lszomoru commented Apr 5, 2024

Closing in favour of #209373.

@lszomoru lszomoru closed this Apr 5, 2024
@gjsjohnmurray gjsjohnmurray deleted the fix-200378 branch April 5, 2024 07:43
@microsoft microsoft locked and limited conversation to collaborators Jun 10, 2024
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.

SCM incoming/outgoing changes show full commit message on hover
2 participants