microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.28k stars 29.31k forks source link

Allow language extensions to provide status information #129037

Closed jrieken closed 2 years ago

jrieken commented 3 years ago

Many language extensions add status bar entries for various information tidbits, like version, language server status, project etc. There is many inconsistencies for how this happens like alignment isn't always in sync with the language name or visibility isn't in sync with the active editor.

We should help extensions do a better job by adding a dedicated "language status" API, some kind of LanguageStatusProvider which we query whenever the editor changes. In its simplest form language status can be text, detail (supporting codicons and md), and severity.

jrieken commented 3 years ago

API sketch

enum LanguageStatusSeverity {
    Information,
    Warning,
    Error
}

declare class LanguageStatus {

    text: string;
    detail: string | MarkdownString;
    severity: LanguageStatusSeverity;

    constructor(text: string);
}

interface LanguageStatusProvider {
    provideLanguageStatus(): ProviderResult<LanguageStatus>;
}

declare namespace languages {
    export function registerLanguageStatusProvider(selector: DocumentSelector, provider: LanguageStatusProvider): Disposable;
}
jrieken commented 3 years ago

fyi @mjbvz

aeschli commented 3 years ago

Good stuff. Not sure about text and detail, I would suggest to go with just message: string | MarkdownString. I imagine this to be shown in a hover over the language mode status bar entry (e.g. 'JSON'). As there can potentially be multiple providers active for a given document, the hover would combine all the messages by concatenating them.

mjbvz commented 3 years ago

Looks promising. Here's the information we'd potentially like to show for JS/TS:

Additionally, it would be nice if we could customize the displayed information per-file instead of showing the same status for all files of given language. For JS/TS, this could be:

If multiple extensions can contribute to the language status, it may also make sense to move some ESLint information into the language status


So my high level questions:

egamma commented 3 years ago

//CC @dbaeumer

aeschli commented 3 years ago

Extensions can always take advantage of the existing vscode API to add additional status bar entries, with command handler, icons, colors, menus etc. That's why I suggest that for the beginning we keep it simple and introduce a LanguageStatusProvider returning a MarkdownString that we can show as hover over the language selector. Thats something we currently can't do through API. Such API is also straightforward to adopt in LSP.

Keep it proposed for a while and I can adopt it in HTML, CSS and JSON to gain some UX experience

jrieken commented 3 years ago

simple and introduce a LanguageStatusProvider returning a MarkdownString.

I think we need more than just a message because we need to show some kind of anchor in the status bar. I initially tried to hijack the language item for that but it has the "Change Language" message and command.

jrieken commented 3 years ago

The current API uses a pull model but if we instead had a push model, we could leave this decision up to extensions like we do for status bar items

Yeah, it might simplify things and prevent repeated LanguageStatusProvider#onDidChange events. Generally we don't know when to ask for language status except when opening an editor. It just needs a nice way to target the files/scope of a certain status. The DocumentSelector could be enough as it support glob patterns and arity.

jrieken commented 3 years ago

This is the updated (and implemented) proposal that uses the push model. I still looking for ways to eliminate the severity, e.g TypeScript being single-file only isn't an error nor warning but merely something "unexpected". Ideally, we find a way to express that, maybe a boolean like affectsUsage or limited

https://github.com/microsoft/vscode/blob/e30d70f9f519780ef07840c77d3ffc57459d9dd2/src/vs/vscode.proposed.d.ts#L3162-L3179

mjbvz commented 3 years ago

Just tested out the changes and like the push model

For the UI, if there are multiple statuses would we want to render them in a list? Perhaps something like:

|===============|
| First Title   |
| details       |
|---------------|
| Second Title  |
| details       |
|===============|
        ˅

A few other ideas:

jrieken commented 3 years ago

Should we add a command to the language status item?

Yeah, 👍 for that - I believe the API will converge to be similar to the status bar API just with us controlling visibility, placement, and arity.

The ASCII art reminds me of how we do notifications and how they are behind the bell. So, one well-known icon which discloses more information. This would give it a clean & uniform look but might be too much hiding.

jrieken commented 3 years ago

This is how it currently looks (UI and API)

Screen Shot 2021-08-20 at 17 10 48
    enum LanguageStatusSeverity {
        Information = 0,
        Warning = 1,
        Error = 2
    }

    interface LanguageStatusItem {
        selector: DocumentSelector;
        severity: LanguageStatusSeverity;
        text: string;
        detail: string; // tooltip!
        command: Command | undefined;
        dispose(): void;
    }

    namespace languages {
        export function createLanguageStatusItem(selector: DocumentSelector): LanguageStatusItem;
    }
daviddossett commented 3 years ago

👋 Here are a couple of additional explorations as I learn more about what's needed here. I've tried a few different versions using the discussion above as input. Any and all feedback welcome! cc @misolori

Option 1

Adds icon before language name

language-status-1

Option 2

Splits status into its own button. Adds counter for potentially multiple messages—not sure if that's needed but thought I'd hint at it here.

language-status-2

Option 3

Similar to option 1. Adds status icon and counter after language name.

language-status-3
mjbvz commented 3 years ago

I like having a separate status bar entry. I'm used to clicking on the language name to change the language mode of the current file, so I'd be careful about changing that behavior

Will showing a warning/error icon be confusing since we use similar icons for diagnostics? I don't have better suggestion for what to show instead

daviddossett commented 3 years ago

@mjbvz Thanks for the feedback! Agreed that we shouldn't change the click behavior. The general idea was for the message to only show on hover. But there are of course challenges there too.

Good question re: icons. I'll see what else we have as options.

bpasero commented 3 years ago

I like Option 3 because that keeps language and status close together, but I share the sentiment that we cannot change the click action.

I would have to play with it myself to see if the hover alone is sufficient. An alternative would be to allow for child elements within a status bar entry to be sensitive to clicks so that you can trigger the language diagnostics with a dedicated click on the warning icon.

dbaeumer commented 3 years ago

I would keep them separate (in terms of the ability of pushing them and have different commands) but visually connected. I do like if the status is in front and not after the language indicator.

aeschli commented 3 years ago

I like them together, with the icon after the language name, if there's a status. I would not put a counter. I think the number of stati is not that interesting as I think for some languages it will be stable number. Instead, a 'dot' could be added if there was a change of status that hasn't been read yet.

daviddossett commented 3 years ago

I've been looking for prior art and thought of Prettier's status bar contribution. Looks like they also combine status and text into one item. Although they don't provide any information on hover. Interesting to note, in any case.

CleanShot 2021-08-24 at 11 53 19@2x CleanShot 2021-08-24 at 11 52 01@2x

In our case, I think it comes down to whether or not the status needs to be an isolated clickable button or not. It feels fairly disconnected to my eyes when split out.

CleanShot 2021-08-24 at 11 55 57@2x
jrieken commented 3 years ago

It feels fairly disconnected to my eyes when split out.

I agree, tho it needs at least two hover areas and likely two click areas. What we could explore is "connected status bar items" which

bpasero commented 3 years ago

We could apply such a solution to SCM status entries too which currently have 2 entries:

image

Though arguably now the benefit is that you can hide either the one or both.

jrieken commented 3 years ago

Removed the padding for this one specific item. Makes it look better but it would be nice if the background effect could show some level of "being connected"

https://user-images.githubusercontent.com/1794099/130813585-e745ec8e-236c-483f-957f-98c293b0a0df.mov

daviddossett commented 3 years ago

What we could explore is "connected status bar items"

Love this idea. I'll explore this today and will post some versions here 👍

daviddossett commented 3 years ago

One idea for a connected status bar item:

Hover on parent element

CleanShot 2021-08-25 at 09 51 24@2x

Hover on child element

CleanShot 2021-08-25 at 09 51 44@2x
jrieken commented 3 years ago

Some updates:

https://user-images.githubusercontent.com/1794099/131149499-40300fb3-0d27-40e8-bb1e-67bbec65de4c.mov

daviddossett commented 3 years ago

Looking good. It seems like the status bar item doesn't yet support the distinction between the parent and children elements yet?

miguelsolorio commented 3 years ago

This oddly reminds me of our split action that has a double hover effect:

CleanShot 2021-08-30 at 15 17 13@2x
daviddossett commented 3 years ago

@misolori good catch! It looks like that has a more elegant solution. Trying something similar below. I never quite loved how I had the child hover being so dark compared to the resting state of the status bar items.

Here the currently hovered element gets the normal hover state we have today. The other element gets a slightly toned down background color hint. Thoughts?

CleanShot 2021-08-30 at 15 31 58@2x
TylerLeonhardt commented 3 years ago

@rjmholt @andschwa FYI this is an API we're introducing to replace status bar items like the one the PowerShell extension contributes.

andyleejordan commented 3 years ago

@rjmholt @andschwa FYI this is an API we're introducing to replace status bar items like the one the PowerShell extension contributes.

This is going to be very nice!

mjbvz commented 3 years ago

I have a draft PR that adopts this for TypeScript and JavaScript: #132015.

Screen Shot 2021-08-31 at 4 32 57 PM

Some feedback on the API itself:

jrieken commented 3 years ago

Should we have an optional shortTitle? When pinning an item, the title is often too long to include in the status bar. With TypeScript for example, we end up with the following:

Yeah, unsure about that. I do agree that it is challenging to cater for both UIs and that's the reason why I added only text and detail. The idea is that they map to StatusBarItem#text and StatusBarItem#tooltip. So, in a way its the same information and just different placements

jrieken commented 3 years ago

@daviddossett Maybe severity (error, warning, and info) should return and rendered with the items as you outlines here https://github.com/microsoft/vscode/issues/129037#issuecomment-903966955. However, for the composite status bar item it could still be two icons - "info" and "screaming info"

TylerLeonhardt commented 3 years ago

@jrieken PowerShell will need to "pin something by default" because the version number is critical to be shown to new users and would be "backcompat" with the current experience which is showing the version. I would guess that we'd wanna do this for TS too for backcompat.

cc @rjmholt @andschwa

daviddossett commented 3 years ago

I spent a few minutes trying out different options this afternoon and landed here. Some quick notes/open questions:

Feedback welcome!

CleanShot 2021-09-01 at 16 48 16@2x
jrieken commented 3 years ago

Trying to clean up the single row format and moved from full on primary buttons to link actions instead.

Love it. Might only weakly differentiate from links in the detail text but it looks and feels much better

https://user-images.githubusercontent.com/1794099/131801457-300c9687-a643-49c0-b1a7-1177558d1bb6.mov

Should the icon button be clickable instead of relying on hover to match the common status bar item behavior?

Yeah, that's on the todo-list

I think the icon should go to the left of the text (at least in LTR languages) to mirror the patterns in Source Control, Problems, etc.

Agreed

I still don't love the usage of the info icon as the "normal" state. It doesn't give any great hints as to that being the location to change something like language version

Also agreeing. For now I brought back the severities but we could simply pick a different renderer for info. Something that's language'ish (whatever that would be) and use dedicated icons (or variants of said icon) for warning and error states

daviddossett commented 3 years ago

Something that's language'ish (whatever that would be) and use dedicated icons (or variants of said icon) for warning and error states

I'll look around to see if we can use something specific to languages here. Ideally we don't have the duplication of the warning/error icons that we use in Problems. May need to create something bespoke for this.

daviddossett commented 3 years ago

Some alternatives to the info icon. These would bias more towards the status itself and less towards any sort of indication of configuration (version, config, etc.)

CleanShot 2021-09-07 at 10 36 56@2x

I don't see any icons that could easily be used for something the languages in our current library. @misolori do you know of any off the top of your head? One idea would be to have something similar to these icon + status patterns found in other icons e.g.

CleanShot 2021-09-07 at 10 35 50@2x
miguelsolorio commented 3 years ago

The only other icon I can think of is the lightbulb, which we do tend to use for "intellisense" and could point to language features. We use that on the website:

CleanShot 2021-09-08 at 05 49 00@2x
justingrant commented 3 years ago

I was pointed here by @dbaeumer after I complained about modal status UI used by vscode-eslint. I'm not an extension author, but I can share my end-user experience below about how I currently use status UIs for different JS/TS-related extensions. Hopefully this will be useful as you evolve this proposal.

First, some context: I work primarily in TS. My apps are a combination of front-end (React) and back-end (Node, AWS Lambda, Serverless Framework). I do a lot of viewing of, debugging into, and occasionally editing of (e.g. for use with patch-package) other packages' code in node_modules.

Here's some use cases and details for how I use language extensions' status UI:

Use Case 1: Why isn't prettier fixing my code when I save a file that I'm editing?

I heavily rely on prettier and format-on-save to fix up my sloppy code formatting. My typical workflow is to write some code, save the file so prettier will fix the formatting, and then continue. When prettier is not working (meaning I save the file and nothing changes), I usually stop what I'm doing and fix whatever is preventing prettier from doing its job. Here's the steps I take to diagnose:

Use Case 2: Preventing prettier from "fixing" manually formatted files

There are often a few files that shouldn't be auto-formatted by prettier. For these files, I turn off the auto-formatter using the vscode-formatting-toggle extension. I look at the status bar and if I see then I'll click it which turns the state to . Then I'll save the file and (hopefully) remember to immediately toggle it back. If I forget, that's (c) in the use-case above.

@jrieken PowerShell will need to "pin something by default" because the version number is critical to be shown to new users and would be "backcompat" with the current experience which is showing the version. I would guess that we'd wanna do this for TS too for backcompat.

+1, because vscode-formatting-toggle could never use this API unless it could be pinned by default, because the whole purpose of that extension is to provide one-click toggling which requires it to always be available on the status bar.

Use Case 3: Dismissing ESLint modal dialogs for problems I can't fix (e.g. when viewing someone else's code)

It's very common to get fatal ESLint failures. Here's a few cases:

In all these cases, the vscode-eslint extension shows a modal dialog that the user must manually dismiss. This is bad because usually the user can't actually fix the problem. Sometimes the cause is someone else's code that I can't fix (cases (a) and (b) above). Sometimes it's a bug in ESLint that I also can't fix, like (c) above. But the common thread is that most ESLint problems are not fixable by most users so the UI should not be modal. Simply telling me "ESLint isn't working on this file" in the status bar would be fine.

As a general guideline, IMHO language extensions should never show modal UI without a really, really good reason, and only if the end-user has a good change of being able to fix the problem. FWIW, these ESLint popups are by far the most frustrating problem I have with VSCode's UI today because it's such a productivity suck to have to dismiss those dialogs every time I open or save a file in some projects.

Use Case 4: Knowing when I can start editing code / viewing problems after a reload

I frequently reload/restart VS Code, either to reload an extension, after an update, to clear stale problems from the Problems pane, or to fix persistent ESLint issues. After a reload, I like to wait until all initialization is done (CSS, JS/TS Language features, folder-open tasks complete, etc.) before I start editing again. I use the status bar UI to let me know when things have settled down.

Use Case 5: Spell Checking

I use the vscode-spell-checker extension for finding spelling typos. This extension has a really nice status bar UI. When there are no problems with the current file, I see this: image

When there are spelling errors, I see this: image

And when the file is ignored I see this, with a nice hover UX: image I like how the icon changes as a subtle hint that I should click the button to open the settings UI for that extension.

FWIW, I also prefer the icon being before the name of the extension (like the spelling extension) instead of after it like the auto-formatting toggler extension.

daviddossett commented 3 years ago

Really appreciate the detailed thoughts @justingrant!

IllusionMH commented 3 years ago

Editor Languages status is available for JS/TS in latest Insiders.

However after "pinning" item it's not clear how to get item back under Language Status group and even if it's related to Language Status. image

I think it would be useful to have some marker/title that item in list (Project config in screen above) is related to Language Status and it will be "hidden" under Language Status group.

bpasero commented 3 years ago

Pushed a change to allow a status bar item to be positioned relative to another one:

https://github.com/microsoft/vscode/blob/ffbe94df0e65ff10ae6db93255199f2ace5b607c/src/vs/workbench/contrib/languageStatus/browser/languageStatus.contribution.ts#L180-L180

And also introduced a new compact property when doing so which allows to move the 2 entries closer to each other:

recording

They can still individually be shown or hidden and are independent status entries as before.

I cannot really get the hover thing to work though, even with the nice adjacent sibling combinator trick which unfortunately only works for the next sibling not the previous one.

Visually I wonder if both entries should simply show the same hover feedback, because the items are quite close together so I feel a individual hover effect on both might not look so nice.

jrieken commented 3 years ago

I cannot really get the hover thing to work though, even with the nice adjacent sibling combinator trick which unfortunately only works for the next sibling not the previous one.

Yeah, I believe it cannot be done in CSS but needs JS. I am also fine with a single hover feedback for "friend items"

bpasero commented 3 years ago

@daviddossett do you have thoughts? I think today in the UX call I saw some designs in action how the language status indicator responds to hover. I wonder what kind of spacing you had in mind for this. Currently with "compact" status entries, I put a 3px padding and no margins to make them appear closer together.

daviddossett commented 3 years ago

Played around with this in Insiders just now. It looks like the combined button hover behavior works when you hover on the language name but not when you hover on the status icon. In any case, great to try this out. I'm ok with us having them separate for now. I may have some other for a sort of nested button that we can explore later.

There was some weirdness with the hover behavior fighting with the click behavior. But IIRC I think @jrieken said he was still working on that.

@bpasero I tinkered with this in dev tools and felt like 3px L/R padding for both the status icon and language item worked well. I think the info icon itself has some extra space on the right however? Visually they still feel somewhat separate compared to something like the Prettier contribution, but we of course need the padding for the button itself.

CleanShot 2021-09-15 at 10 01 48@2x

A related question: how do you feel about the hover treatment when compared to other popovers like notifications and feedback. Both use more of a formal popover w/ shadow and status bar beak. My first impression was that it was a "one of these is not like the other" situation.

CleanShot 2021-09-15 at 09 53 57@2x CleanShot 2021-09-15 at 09 55 24@2x
daviddossett commented 3 years ago

Another advantage to using the default item hover styling (i.e. not going darker) is that we're less likely to run into issues like this on other themes. Note that the background color doesn't change on hover here:

CleanShot 2021-09-15 at 10 08 21@2x
Tyriar commented 3 years ago

Seeing this just now I think we should also consider merging them into a single status bar item. So instead of the info icon it would just say "TypeScript", and you could hover to see "Select Language Mode" above the pinnable language status entries. @eamodio tells me he brought this up before and the push back against that is that then it's a hover only action, I don't see why that's a problem though.

As for the warning one, that could still show the warning-color icon, just it would be closer to the language name, only separated by ~a single space, making it much more obvious that the warning is related to the language.

justingrant commented 3 years ago

A bunch of feedback and questions below:

One hover vs. separate

For answering questions like "should there be one hover or multiple?", I'd suggest describing the step-by-step user actions for the various use cases involved, to make sure we don't miss any. For example:

I probably agree with @Tyriar that having a single hover (not separate hovers on icon vs. text) will be clearer UX. Without seeing the cases and user flow for each, it's hard to know for sure though. A split UX may make some cases much easier.

Simple Extensions

One case to keep on your radar is simple extensions like vscode-formatting-toggle. These extensions put 100% of their important UX (name, status, and click to change status) inside the status bar don't have any follow-up actions in the hover, just text. For complex extensions with multiple actions, the proposed changes discussed above seem like a big win, but need to make sure that it doesn't make UX worse for simple extensions.

It's an interesting question of when icons should be shown for non-warning/non-error situations. I'd suggest leaving it up to the extension. Extensions whose status changes frequently as the user edits code (e.g. ESLint, Prettier) probably will want to keep the icon showing all the time to reduce distracting movement in the status bar as the user types. Extensions like TS, where status changes rarely and/or only in response to an explicit user action in the status bar, probably will want to hide the icon unless there's a problem.

Pinning

I admit I don't understand how pinning will look and act. Would be good to see some screenshots/videos for pinned vs. un-pinned states, and for the transition between those states. A few pinning questions:

Icon Questions

Hover Behavior

it's a hover only action, I don't see why that's a problem though.

Is using VS Code on a touch-only device a supported use case? If so, then would hover-only actions would need a tap alternative.

Does clicking on the status bar item cause the hover UX to come up and stay up until you set focus away? If so, that would need to be customizable to avoid breaking the "simple extensions" case noted above.

bpasero commented 3 years ago

@daviddossett if you are using insiders you are not on my solution yet, you need to wait for todays insider or try running out of sources.

As for sizing, I used to have 2px padding but can change to 3px padding for each item that is close to each other, so we end up with 6px gap from indicator to language picker:

image

image

Btw, I am yielding from any discussion around how the language status indicator should look like because @jrieken owns this component. I am only providing the support for showing status bar entries close to each other. The current solution has the advantage that you can have language status indicator visible but language mode hidden, or show both or show either one. On top of that, the language status indicator is clickable to show the hover immediately.