search

zanza00 / random-theme-switcher

a Visual Studio Code extension that chooses a theme for you
https://marketplace.visualstudio.com/items?itemName=zanza00.random-theme-switcher
8 stars 0 forks source link

Feature request: Remove no longer installed themes from the themeList #21

Open ghost opened 4 years ago

ghost commented 4 years ago

Hey @zanza00 !😊👋🏼

While developing an extension i learned about how to use linters: Basically, you can run a task to parse a file in order to provide Diagnostic objects. With Diagnostics you can show warnings, errors or information in the Error Panel. Plus, providing a range you can make VSC draw squiggles in documents.

Having said that: What about squiggling themes in the themeList that are no longer installed ?

While writing this another approach came in my mind 😂: What about automatically fix the themeList at startup and at onDidChangeConfiguration ? After a removal/s a message popups saying: "Removed ${n} no longer used themes from the themeList".

What do you think about it ?

zanza00 commented 4 years ago

We should definetely have a linter, we shallots continue this discussion in #13

Back to the main topic, when do you think is the best moment to check for removing the un installed themes?

ghost commented 4 years ago

Good evening @zanza00 ! This time it's me that i missed your message ! 😅

However, this issue deals with user's backend, not our dev's one.

I have found that vscode.extensions.onDidChange notifies extensions when changes occur, so if we keep an installedThemeList cache we can check the list and detect uninstalled themes.

I suppose that an uninstalled theme is basically a string that doesn't have any theme associated with it. So the best moments are : 1) at startup: so the _themeList don't get cluttered of useless entries 2) onDidChangeConfiguration, because when users edit the themeList in the settings.json could potentially add junk

Now, if we would be very smart we could: 1) check at startup and at onDidChangeConfiguration the randomThemeSwitcher.themeList 2) purge our instance _themeList (so no junk get selected by our switchTheme) 3) display a warning box asking the user if he want us to clear the stored list (randomThemeSwitcher.themeList) from these items 4) allow him to reply "No" and fix the list manually, or reply "Yes" and purge the list in the settings.json 5) the linter so he can clearly see what's wrong in the list 5.1) a "did you mean: themeName ?" code action feature could be interesting to be integrated into this linter.

Having said that: I'm afraid a linter could add extra weight to the extension without bringing any real benefits to users.

The Best Code is the Code you don't write

What do you think ?

zanza00 commented 4 years ago

Now I understand what you meant by the linter 😅

IMHO a linter is too complex right now.

onDidChangeConfiguration, because when users edit the themeList in the settings.json could potentially add junk

this is a very good idea, we could report the "junk" as not recognized in a notification. Also, this would pave the way for said linter if we are smart about the linter function.

One complain that I have about your plan is the purge of all _themeList, it seems too much for a wrong entry or am I missing something?

ghost commented 4 years ago

Hello ! I have been in holidays.😎🍸 Now i'm back and i can see more clearly some scenarios:

Finally, the proposed solution is :

  1. Check at startup, onDidChangeConfiguration and vscode.extensions.onDidChange the randomThemeSwitcher.themeList integrity
  2. Purge our instance _themeList in order to keep only the valid theme names
  3. Warning the user about the "junk" detected in the randomThemeList
  4. Allow him to reply "No" and fix the list manually, or reply "Yes" and purge the list in the settings.json
  5. The linter so he can clearly see what's wrong in the list and decide what to do.
ghost commented 4 years ago

Hi, I opened a PR addressing this issue.

I found vscode.extensions.onDidChange to be hidden but reachable, so i successfully managed to listen to it. The only drawback is that you can't know from the event if an extension is uninstalled or simply deactivated. But not a big issue: the extension will behave well anyway.