Skip to content

Consider moving the PowerShell ISE theme into a separate extension #1943

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

Closed
jrieken opened this issue May 3, 2019 · 14 comments · Fixed by #3372
Closed

Consider moving the PowerShell ISE theme into a separate extension #1943

jrieken opened this issue May 3, 2019 · 14 comments · Fixed by #3372
Labels
Area-UI Issue-Enhancement A feature request (enhancement). Up for Grabs Will shepherd PRs.

Comments

@jrieken
Copy link

jrieken commented May 3, 2019

  • install this extension
  • 🐛 theme switches

This is really uncool - while I can understand your excitement about your theme you should understand that not everyone is equally excited about it. The extension already pops up a notification message and that would be a much more subtle place for "selling" your theme

Screenshot 2019-05-03 at 12 39 41

@jrieken
Copy link
Author

jrieken commented May 3, 2019

Oh, wait this is vscode itself...

@jrieken jrieken changed the title Don't force your theme on me Consider moving the PowerShell ISE theme into a separate extension May 3, 2019
@jrieken
Copy link
Author

jrieken commented May 3, 2019

Ok, I have updated the title and filed an issue on our (vscode) side. However, I think separating the theme from the language extension is beneficial and would avoid the confusion here

@rjmholt
Copy link
Contributor

rjmholt commented May 6, 2019

Yeah we've been experiencing the ISE theme setting itself in dev builds -- it's definitely not deliberate and as far as we know isn't the result of a code change in the extension.

@corbob
Copy link
Contributor

corbob commented May 6, 2019

This was definitely a change in Code that introduced this. I mentioned in the code issue, but I think it started occurring around when they removed the reload on extension install.

I think from Code's perspective it makes sense that when you're installing an extension with a theme, you likely want to enable that theme (since PowerShell is the only extension I'm aware of that provides a theme among other offerings). If it's decided to extract the theme from the extension, then I guess the biggest question would be: what's the best way to extract the theme and not cause issues for users of the theme? Would it be best to extract it to it's own theme extension, and somehow alert users that are using it, then after a few months remove it from the extension?

@rjmholt
Copy link
Contributor

rjmholt commented May 6, 2019

I think it's worth noting that it's a bit of a usability/simplicity downgrade to demerge the ISE theme. I don't want to speak for them, but I think many PowerShell extension users expect a batteries-included experience with the PowerShell extension and that includes the ISE theme.

@TylerLeonhardt
Copy link
Member

I agree with @rjmholt. We've also heavily preached (in talks and docs) that the ISE theme comes with the PowerShell extension. This would be a pretty big breaking change for us that would affect a lot of people who depend on this theme.

And it sounds silly to put a theme so high on the "critical features" list but I've given talks where people give a standing ovation for this theme.

@sandy081
Copy link

sandy081 commented May 7, 2019

Well, this picker is very helpful for theme extensions but the powershell extension is trying to compose various (language features, themes etc) features into one extension. It would be a good design if they can split into two extensions one with language features and other with theme as they are not related. This will also help with remote set up - https://code.visualstudio.com/blogs/2019/05/02/remote-development#_managing-extensions.

You can also have a pack that can compose these extensions for those who want to have also themes. This will give flexibility to users to install either language features or both and do not force to install the theme.

@rjmholt
Copy link
Contributor

rjmholt commented May 7, 2019

We'll investigate extension packs, but honestly we've already had to manage quite a bit of complexity with the extension and it's felt like we've needed to fight VSCode a lot of the way. I'm reluctant to sign up for more; things are already quite complex.

The PowerShell extension composes all kinds of features, like:

  • Language server
  • Debugger
  • REPL
    • An editor object model with pluggable architecture
  • CodeLens
    • Test running
    • Reference analysis
  • Script Analysis/linting
  • The ISE theme

The design here tries to be modular. We include and configure things by default where they make sense and just ship them as optional features otherwise. The ISE theme is in that second category; it's a small addition that's easy to tack on. And if VSCode didn't switch to it by default it would be a completely benign addition. Just like the suites of PSScriptAnalyzer rules we ship but need configuration to activate, the object model APIs that only get used if you write PowerShell for them or the CodeLenses that only activate if you have certain modules installed.

We already maintain:

These are separate components for reuse; PowerShellEditorServices is the reusable backend, EditorSyntax is a textmate grammar used for highlighting in VSCode but also in GitHub and other editors.

Modularising the ISE theme would provide no reuse. Another extension represents significant overhead to us as developers in terms of maintenance and release work and makes life harder for our users, who already grapple with a large amount of configuration and are confused enough by the architecture we employ.

A theme strikes me as an inert JSON payload, one which should be harmless to ship as part of an extension bundle. Spinning out what is a pretty small component (< 200 lines) feels like a high price to pay because of a new auto-magic behaviour in VSCode (which PowerShell in its history has also felt the pain of -- there are plenty of issues in PowerShell because it tried to guess users' intent and managed to break others in doing so; such is the challenge of creating a platform).

@sandy081
Copy link

sandy081 commented May 8, 2019

@rjmholt

but honestly we've already had to manage quite a bit of complexity with the extension and it's felt like we've needed to fight VSCode a lot of the way.

Sorry for that, but I am not sure about this. I am only suggesting that it is not a good idea to mix up language specific features and personalised features. Having them as separate extensions will give better user experience. For example, I might not be interested with ISE theme but it still comes in my theme picker.

The design here tries to be modular. We include and configure things by default where they make sense and just ship them as optional features otherwise.

Agreed. But too much customisation can overwhelm users instead you can categorise your features and provide them as multiple extensions.

By grouping all features under single extension might also make it hard for users to discover features. For example, I cannot find this extension if I try to look for ISE theme. If I look for powershell theme, I get this extension which did not talk about the theme.

@rkeithhill
Copy link
Contributor

I might not be interested with ISE theme but it still comes in my theme picker.

I have to agree with this. I've never wanted an ISE theme. I left ISE in the dust long ago. :-) As long as its presence was "hidden" it didn't bother me but now it's not so hidden.

@TylerLeonhardt TylerLeonhardt added Area-UI Issue-Enhancement A feature request (enhancement). labels May 8, 2019
@TylerLeonhardt
Copy link
Member

We will have to invest the time to:

  1. include a warning in the extension that says "the theme will be separated to a new extension"
  2. evangelize heavily
  3. set up the release process for yet another extension

Which is fine, but it'll take work. I don't think we've received enough customer feedback to add this on our todo list today (keep in mind, we are about 0.5 devs), but we will keep the issue open to capture the work.

@rjmholt rjmholt added Up for Grabs Will shepherd PRs. and removed Triage labels May 8, 2019
@andyleejordan andyleejordan self-assigned this Feb 17, 2021
@andyleejordan andyleejordan added this to the Consider-vNext milestone Feb 17, 2021
@andyleejordan andyleejordan removed their assignment Apr 28, 2021
@andyleejordan
Copy link
Member

Can anyone confirm my suspicion that this is no longer an issue? That is, whenever I've installed the VSIX lately in setup and fresh VS Code instances, the theme no longer auto-applies.

@corbob
Copy link
Contributor

corbob commented May 25, 2021

Can anyone confirm my suspicion that this is no longer an issue? That is, whenever I've installed the VSIX lately in setup and fresh VS Code instances, the theme no longer auto-applies.

Can confirm that this seems to no longer happen. It looks like the UI added this option to set the theme once it's been installed:
2021-05-25_11-31-43

Picking a random extension that is just a theme, it pops up the choice as soon as it's installed:
2021-05-25_11-33-25

So it looks like code changed their behaviour to only change if it's just a theme shipped with the extension 😁

@ghost ghost added the Needs: Maintainer Attention Maintainer attention needed! label May 25, 2021
@andyleejordan
Copy link
Member

Hooray! I'm simply going to update our own docs and make sure it's called out in the readme that you get the ISE theme with this extension, but that you must (finally!) manually activate it if you want it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-UI Issue-Enhancement A feature request (enhancement). Up for Grabs Will shepherd PRs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants