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

Can't allow a before disallowed integrated terminal #23362

Closed
dbaeumer opened this issue Mar 28, 2017 · 5 comments
Closed

Can't allow a before disallowed integrated terminal #23362

dbaeumer opened this issue Mar 28, 2017 · 5 comments
Assignees
Labels
debt Code quality issues terminal Integrated terminal issues
Milestone

Comments

@dbaeumer
Copy link
Member

dbaeumer commented Mar 28, 2017

Steps to Reproduce:

  1. customize a shell workspace settings using "terminal.integrated.shell.windows": "C:\\WINDOWS\\Sysnative\\bash.exe"
  2. Disallow the shell

Try to allow the shell. The only way I found is to use the dev tools and delete the key.

@dbaeumer dbaeumer added the terminal Integrated terminal issues label Mar 28, 2017
@Tyriar
Copy link
Member

Tyriar commented Mar 28, 2017

Any suggestions how to surface this without annoying the user? (how would disallow differ from cancel?)

@Tyriar Tyriar added the info-needed Issue requires more information from poster label Mar 28, 2017
@dbaeumer
Copy link
Member Author

The problem here is that after a disallow the dialog doesn't show up anymore. So no way to trust the workspace later without using the dev tools and editing local storage

@Tyriar
Copy link
Member

Tyriar commented Mar 29, 2017

@dbaeumer yes, the workarounds being moving the folder or changing local storage. The alternative is using user setting to drive this instead of local storage. Seems a bit messy doing that since paths generally aren't portable across machines and it's workspace-specific config in your user settings.

Since the use case of a user re-enabling this at a later time seems to be so small I don't think it's worth changing this. The usage numbers for workspace shell's were really small when I looked at them so the chance that a user would choose to disallow and later make a conscious decision to allow seems really slim. Closing as designed for now, will listen for feedback.

@Tyriar Tyriar closed this as completed Mar 29, 2017
@Tyriar Tyriar added *as-designed Described behavior is as designed and removed info-needed Issue requires more information from poster labels Mar 29, 2017
@dbaeumer
Copy link
Member Author

Strange. I was forced to do this for PHP :-). I simply added a context sensitive command to clear local storage.

@Tyriar
Copy link
Member

Tyriar commented Mar 29, 2017

@dbaeumer maybe a command would be good, still not for this version. The multi-root discussion we're having at the moment may actually impact this.

@Tyriar Tyriar reopened this Mar 29, 2017
@Tyriar Tyriar added debt Code quality issues and removed *as-designed Described behavior is as designed labels Mar 29, 2017
@Tyriar Tyriar added this to the April 2017 milestone Mar 29, 2017
@Tyriar Tyriar modified the milestones: April 2017, May 2017 Apr 23, 2017
@Tyriar Tyriar closed this as completed in 5f5df7c May 14, 2017
marckassay pushed a commit to marckassay/vscode that referenced this issue May 15, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues terminal Integrated terminal issues
Projects
None yet
Development

No branches or pull requests

2 participants