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

Add support for disabled extensions #838

Closed
wants to merge 7 commits into from
Closed

Add support for disabled extensions #838

wants to merge 7 commits into from

Conversation

lhnrd
Copy link

@lhnrd lhnrd commented Apr 9, 2019

Short description of what this resolves:

Takes account disabled extensions when uploading or downloading the gist file.

Changes proposed in this pull request:

  • CreateExtensionList now uses code --list-extensions to get enabled and disabled extensions;
  • When download executes it verifies the extension object's enabled property . If false calls code --disable-extension with the extension name.

Fixes: #143

How Has This Been Tested?

Tested on a Macbook with VSCode.

  • Open code-settings-sync project;
  • Disable an extension;
  • While debugging (F5) inside this branch execute Upload / Update settings;
  • Then check in your gist if the current extension is disabled;
  • Leave the debugging mode and uninstall the same extension;
  • Open the debugging mode and execute Download settings;
  • Check if it installs the extension and disables it automatically.

Checklist:

  • I have read the contribution guidelines.
  • My change requires a change to the documentation and GitHub Wiki.
  • I have updated the documentation and Wiki accordingly.

@lhnrd lhnrd mentioned this pull request Apr 9, 2019
3 tasks
@ohmyshell
Copy link

Please look into this @shanalikhan. Thank you for this extension. Also thank you for your effort @lcsrinaldi.

@shanalikhan
Copy link
Owner

Give me some time , a two weeks or three.
Need to merge other code PR before reviewing and merging this.

@shanalikhan shanalikhan changed the base branch from v3.2.9 to v3.3.0 April 19, 2019 10:06
@Kurtz1993
Copy link

Any updates on this PR status? 😃 I'd love to have this feature.

@shanalikhan shanalikhan added this to the v3.1.0 milestone May 28, 2019
@shanalikhan shanalikhan modified the milestones: v3.1.0, v3.4.0 Jun 6, 2019
@shanalikhan
Copy link
Owner

microsoft/vscode#15466
Please upvote and comment over there. Using API instead of CLI is more feasible.

Settings Sync use API to install/uninstall all the extensions from the v3.3.0

@richardsavio
Copy link

Wouldn't it be better if we have support for this using the CLI available while we wait for API to materialize? Also the vscode team hasn't assigned a proper milestone for the feature request. So it is not a priority for them right now. Does the PR need any changes to be useful enough to be merged? Would be glad to give it a try.

@auxves
Copy link
Contributor

auxves commented Jun 7, 2019

@richcocoa The problem is that we can't easily find the CLI. There are so many variants of VSCode that it would be inefficient/impossible to hardcode them. That's why we're trying to use the API.

@lhnrd
Copy link
Author

lhnrd commented Jun 7, 2019

I don't understand what's the problem of using the CLI.

As you can see here https://github.com/shanalikhan/code-settings-sync/blob/v3.2.9/src/service/pluginService.ts#L302

Correct me if I'm wrong, but the code is already using the command line to install other extensions.

@auxves
Copy link
Contributor

auxves commented Jun 7, 2019

@r-inaldi Yes, but coverage is spotty with the CLI method. It's a pain getting the code executable when there are versions of code, like code-server and vscodium, that have different binaries. Also, Windows, macOS, and Linux all have the executables in different places.

@shanalikhan
Copy link
Owner

shanalikhan commented Jun 8, 2019

@r-inaldi

Actually, CLI based commands are going to remove in the upcoming version and we are replacing with vs code API because CLI commands were not working with OSS versions for code.

I have asked code team, lets see what they respond, else what i think for the temporary solution will be to use CLI commands only for enabling / disabling extensions and mentioning the limitation that CLI commands wont work for code OSS versions.

I am keeping this PR open.

@shanalikhan shanalikhan changed the base branch from v3.3.0 to v3.4.0 June 24, 2019 21:09
@shanalikhan
Copy link
Owner

shanalikhan commented Jun 25, 2019

@r-inaldi

We can use CLI based commands for disabling and enabling the extensions.

We can release this PR in v3.4.0. Currently the CLI based methods are removed from the code in favor of API to install / uninstall extensions.

This feature will be disabled by default using Global Settings Configuration and needs to be enable to allow user to disable/enable extension with the warning that it doesn't support OSS versions.

Can you fix this PR and let me know when its ready to merge.

@shanalikhan shanalikhan modified the milestones: v3.4.0, Backlog Jun 28, 2019
@shanalikhan shanalikhan closed this Jul 3, 2019
@shanalikhan
Copy link
Owner

Let me know when you fix it, I will merge the PR.

@lhnrd
Copy link
Author

lhnrd commented Jul 3, 2019

@shanalikhan

Hey there, not sure what you want me to fix in this PR. Merge conflicts?

Also, any reason to close the pull request? Should I open a new one?

@shanalikhan
Copy link
Owner

We are now using API to perform functions that were previously using VSCode Command lines interface. (e.g installing extensions). Because we were unable to identify the code command line path for code OSS versions.
If you look into the latest version you will see, Settings Sync doesnt use command line to install extensions.

Enable / Disable Extension doesn't have API ( that i have asked on microsoft/vscode#15466) hence you need to introduce the command line in extension back again to perform enable / Disable extensions only.

Let me know if you have any confusion. You will find the code related to running command lines in releases prior to v3.3

@lhnrd
Copy link
Author

lhnrd commented Jul 3, 2019

Oh cool, thanks for explaining! I'll make this changes when I have some time.

Do you also want me to add an option to enable this in the extension settings?

@shanalikhan
Copy link
Owner

This feature will be disabled by default using Global Settings Configuration and needs to be enable to allow user to disable/enable extension with the warning that it doesn't support OSS versions.

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants