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

Introduce tree item highlights #61482

Merged
merged 4 commits into from
Oct 23, 2018
Merged

Introduce tree item highlights #61482

merged 4 commits into from
Oct 23, 2018

Conversation

sandy081
Copy link
Member

Introduce tree item highlights in Tree item api

Fixes #61313

@sandy081 sandy081 requested a review from jrieken October 22, 2018 11:48
@sandy081 sandy081 self-assigned this Oct 22, 2018
@sandy081 sandy081 added this to the October 2018 milestone Oct 22, 2018
@sandy081
Copy link
Member Author

@jrieken Please review the API changes

Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

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

Looking good but please move to vscode.proposed.d.ts so that we can still make changes here. Also, we don't have a way yet to define the highlight style. I think the default is a color and font weight change but I am looking for a background color change...

/**
* Ranges in the label to highlight.
*/
highlights?: { start: number, end: number }[];
Copy link
Member

Choose a reason for hiding this comment

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

We have been using tuples instead of objects so far, e.g valueSelection and ParameterInfo#label use [number,number]

/**
* Label describing the [Tree item](#TreeItem)
*/
export interface TreeItemLabel {
Copy link
Member

Choose a reason for hiding this comment

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

Having this as a separate type means, this will only work when the label is a string, right. Or putting it the other way, highlights will not work when the label is an Uri, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. You can only provide highlights to the labels you contribute. Otherwise it is not correct to allow providing highlights to computed labels.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you can only have highlights for labels you contribute, otherwise it is not correct to allow them for computed labels.

@sandy081
Copy link
Member Author

@jrieken Updated

/**
* Ranges in the label to highlight.
*/
highlights?: [number][number][];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
highlights?: [number][number][];
highlights?: [number,number][];

Copy link
Member

Choose a reason for hiding this comment

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

I think like that. Not even sure what [number][number][] means?

Copy link
Member Author

@sandy081 sandy081 Oct 22, 2018

Choose a reason for hiding this comment

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

I also do not know 😄 . It was a typo. Fixed.

@eamodio
Copy link
Contributor

eamodio commented Oct 23, 2018

@sandy081 @jrieken what is the intent of this api? Is this something where the extension decides on what should be highlighted? Like for instance if a filter/search box were available? Rather than something like the quickpick which searches and highlights without extension interaction?

@sandy081
Copy link
Member Author

@eamodio This is to allow extension to highlight displayed labels. VS Code will not do anything automatically. Please see #61313 for Use case. In short if there is a search kind of view, you can highlight matches.

@sandy081
Copy link
Member Author

@jrieken Fixed highlights type. Please check. Thanks.

Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

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

👍

@sandy081 sandy081 merged commit 6baf2fd into master Oct 23, 2018
@sandy081 sandy081 deleted the sandy081/61313 branch October 23, 2018 08:18
@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.

TreeItem should support highlights
3 participants