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

Support workspace-level terminal.integrated.cwd in multi-root workspaces #42661

Closed
Tyriar opened this issue Jan 31, 2018 · 14 comments
Closed

Support workspace-level terminal.integrated.cwd in multi-root workspaces #42661

Tyriar opened this issue Jan 31, 2018 · 14 comments
Assignees
Labels
*duplicate Issue identified as a duplicate of another issue(s) feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities terminal Integrated terminal issues
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Jan 31, 2018

No description provided.

@Tyriar Tyriar added help wanted Issues identified as good community contribution opportunities feature-request Request for new features or functionality terminal Integrated terminal issues labels Jan 31, 2018
@Tyriar Tyriar self-assigned this Jan 31, 2018
@vrachels
Copy link

sorry, as discussed in #39826, I think there's still some failure to communicate properly.

I'm not sure what 'folder setting' implies, I haven't seen that reference, nor does search come up with anything obvious. Do you mean workspace setting, or is this something else? If you mean workspace setting, then yes, absolutely, definitely, why wouldn't it? If it doesn't currently, then yes, please.

Specifically, the bug I was reporting boils down to trying to set a terminal path for a specific workspace, because the current behavior of 'figure it out based on the current/last opened file' is not acceptable in our environment, and it doesn't look like that feature will change.
Checking in the workspace setting with the cwd set is expected behavior, with a value that is a relative path to the .code-workspace file - either via a variable (#31677) or just assume a relative path (including '.') is from ${workspaceFolder}

@Tyriar
Copy link
Member Author

Tyriar commented Feb 1, 2018

@vrachels workspace settings become folder settings when you're in a multi-root environment, this screenshot illustrates this best:

screen shot 2018-01-31 at 4 22 04 pm

Do you "save your workspace as" and have the terminal.integrated.cwd setting defined in the .code-workspace? You want the cwd to be relative to the .code-workspace? I don't think we do that for anything currently, I believe ${workspaceFolder} refers to the current active folder concept (last opened file) elsewhere.

@Tyriar
Copy link
Member Author

Tyriar commented Feb 1, 2018

The reason we don't use the folder that .code-workspace is in is because it's nearly always hidden away in some hidden folder:

screen shot 2018-01-31 at 4 27 55 pm

Launching a terminal relative to that folder isn't particularly useful.

@vrachels
Copy link

vrachels commented Feb 1, 2018

As, I see and understand some of the discrepencies now.
I think as multi-root becomes more common, that use case will change. We took advantage of it as soon as it was available in insiders. In order to share the multi-root environment, it is my understanding that the code-workspace file also needs to be shared (much like a msdev .sln file is usually checked in), and thus lives in the repository. I wasn't actually aware that the non-multiroot used the same file in a hidden location. Is there is a different intention for setting up a shared multi-root environment?

I think I was also getting workspaceRoot and workspaceFolder confused. I saw references to workspaceRoot being deprecated, and wrongly assumed general equivalence. I see now that this the continually modifying value based on the current file that has been getting discussed as the 'calculated root'

Back to the title issue: yes, we 'save workspace as,' and define the terminal.integrated.cwd setting there. The expected behavior was that I could then share that file, and other developers who sync and open the project will get their default terminal to a consistent relative location to run various commands.
Not all developers are created equal (most have way more artistic talent than I), and ensuring the consistency of instructions is important.

I could see a use case for the feature request as specified in the title. (ie, a different relative cwd based on the ${workspaceFolder} that was calculated), but I am definitely asking for cwd to be able to be specified relative to the .code-workspace file (via workspace settings, not necessarily folder settings)

@Tyriar
Copy link
Member Author

Tyriar commented Feb 1, 2018

I wasn't actually aware that the non-multiroot used the same file in a hidden location.

To clarify, non-multiroot don't have a .code-workspace file. multiroot by default have a .code-workspace hidden away in some folder, it's only when you explicitly save the (multiroot) workspace somewhere that it goes to a reasonable location chosen by the user.

it is my understanding that the code-workspace file also needs to be shared (much like a msdev .sln file is usually checked in), and thus lives in the repository.

This is good, it's the intent behind being able to save a .code-workspace and allowing the roots to be relative.

The expected behavior was that I could then share that file, and other developers who sync and open the project will get their default terminal to a consistent relative location to run various commands.

I think this was the missing piece in my understanding of your problem, it's only an issue for you because you're sharing the .code-workspace.

@bpasero are you aware of anyone coming across this issue in any other components? Specifically, because the .code-workspace is checked in and shared with the team, an absolute terminal.integrated.cwd value will not work as developers have different file system layouts, once shared everything needs to be relative. So I see 3 solutions here, none of them are particularly nice imo:

  • Make terminal.integrated.cwd work relative to the .code-workspace, not the "active workspace root". This is inconsistent with how everything else works AFAIK
  • Provide a setting to toggle the above behavior. This is extremely specific and would likely seldom be used/discovered.
  • Make terminal.integrated.cwd work in folder setting scope, and define it for every single root to go to the correct single location relative to root.

@Tyriar Tyriar added under-discussion Issue is under discussion for relevance, priority, approach and removed feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities labels Feb 1, 2018
@Tyriar Tyriar changed the title Allow terminal.integrated.cwd to work as a folder setting Allow terminal.integrated.cwd to be relative to .code-workspace Feb 1, 2018
@vrachels
Copy link

vrachels commented Feb 1, 2018

another option similar to 1, but probably more palatable: un-deprecate workspaceRoot or create another variable that means the code-workspace location (see below). Then unblock/implement/fix #31677

another feature that could be used instead, to avoid the usage of workspaceRoot and in possibly other situations, is to be able to specify via a variable a specific workspaceFolder. (ie, workspace contains foo/folder1 and bar/folder2. in some kind of setting, give them names, or just identify them as their array entries). Then with 31677, you could specify via the variable expansion ie "${workspaceFolders[0]}

@bpasero
Copy link
Member

bpasero commented Feb 2, 2018

@Tyriar you should maybe connect with debug team who introduced debug launch configuration within the code-workspace file and with that variables that resolve against a specific root. For that a new variables syntax is used that allows to scope to a specific root folder. Maybe something like that could be used for the CWD setting as well? E.g. ${workspaceFolder:<name of folder>}

@Tyriar Tyriar added feature-request Request for new features or functionality and removed under-discussion Issue is under discussion for relevance, priority, approach labels Feb 21, 2018
@coyoteecd
Copy link

So, is this going to be fixed? I tried out the multi-root workspace feature on our project and have to go back because of terminal.integrated.cwd not working anymore. In a team setting, the .code-workspace files are in the source control.

Supporting ${workspaceFolder:<name of folder>} is consistent with the launch settings, which are also shared across the team via the workspace file.

@Tyriar
Copy link
Member Author

Tyriar commented Oct 8, 2019

Variable resolution now works in the cwd setting, however that only applies when this command is triggered:

image

The normal picker when in a multi-root workspace (ctrl+shift+`) will always open in the selected root. If the cwd is set for the user or workspace it should be used here:

return this.terminalService.createTerminal({ cwd: workspace.uri });

@Tyriar Tyriar added the help wanted Issues identified as good community contribution opportunities label Oct 8, 2019
@Tyriar Tyriar changed the title Allow terminal.integrated.cwd to be relative to .code-workspace Support workspace-level terminal.integrated.cwd in multi-root workspaces Oct 8, 2019
@petervandivier
Copy link

For my two cents, just cause this cost me my morning trying to figure out what was going on...

Intuitively when opening a multi-root workspace with no open editors, I would expect the terminal to launch to the root of the top-level folder (I could've sworn I read that as the default behaviour somewhere).

Certainly I know now what the expected behaviour is, but figuring it out caused me a small amount of extra grief in part because at the time I was using a custom shell prompt that does not show the current working directory.

At a minimum it would be nice to know where the "last used file" information is cached just for the trivia of it (or for debugging). I did not see anything immediately helpful in ~/.vscode, /Applications/Visual Studio Code.app/, or ~/Library/Application Support/Code/Workspaces 😕

@Tyriar
Copy link
Member Author

Tyriar commented Oct 10, 2019

I would expect the terminal to launch to the root of the top-level folder

There isn't really a concept of "top-level folder" as all folders are on the same level. There is the active workspace command mentioned in #42661 (comment) which you can set a custom keybinding to if you prefer that, this will open it in the workspace of the last file you opened. This used to be the default behavior but it changes to the quick pick as I got a lot of complaints about it.

I think it's cached in sqlite as part of the workbench state so you wouldn't be able to view it.

@petervandivier
Copy link

petervandivier commented Oct 11, 2019

There isn't really a concept of "top-level folder" as all folders are on the same level

I understand that now, but much of the UI and underlying config appears to contradict that intuitively (even though this intuition is incorrect as you've mentioned).

For example - when dragging to re-order root folder ordering, this caution pops up

image

This would seem to place importance on the "ordering" of the co-equal roots.

As well, when launching the "ctrl+shift+`" picker, the choice of workspace is listed in top-down order matching the *.code-workspace json document

image

mkdir foo bar baz
touch foo/foo.sh bar/bar.ps1 baz/baz.c
echo '{"folders":[{"path":"baz"},{"path":"foo"},{"path":"bar"}],"settings":{}}' > foobar.code-workspace
code foobar.code-workspace

@petervandivier
Copy link

petervandivier commented Oct 11, 2019

I think it's cached in sqlite as part of the workbench state so you wouldn't be able to view it.

Almost, but perhaps not quite. You can access and query the sqlite db directly (in my case at ~/Library/Application Support/Code/User/workspaceStorage/*/state.vscdb). It looks like the relevant key may be workbench.explorer.treeViewState, but I'll need to play with it a bit for funsies before making too many assumptions

ty! 😄

@Tyriar
Copy link
Member Author

Tyriar commented Aug 27, 2021

This is the same request as #42165 at this point

/duplicate

@Tyriar Tyriar closed this as completed Aug 27, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Oct 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*duplicate Issue identified as a duplicate of another issue(s) feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities terminal Integrated terminal issues
Projects
None yet
Development

No branches or pull requests

6 participants
@bpasero @Tyriar @petervandivier @vrachels @coyoteecd and others