folders in `.dav/local.list.txt` should not be opened in a new window

tjyuyao commented 2 years ago

Maintaining a local favorite list is beneficial for version control. It gives a compact view with a custom order of files, which is good for big complex projects. But clicking a folder should not just open it as a new "project", but do something like "reveal in explorer sidebar", since the project itself is already open.

oleg-shilo commented 2 years ago

Do I understand you correctly that the proposed behaviour is to:

open folder/workspace on clicking <Open Folder> node if the folder is not already opened
do nothing on clicking <Open Folder> node if the folder is already opened

tjyuyao commented 2 years ago

Thanks for your reply. This is a great plugin.

open folder/workspace on clicking node if the folder is not already opened

YES. do nothing on clicking node if the folder is already opened

The condition would be "the folder is already opened in the workspace" or "the folder is a subdirectory of the opened workspace".

The behavior "do nothing" is acceptable and good enough. But if you right-click on a tab title of opened file editor, there will be an option named "Reveal in sidebar", which finds and focuses the current file in the exporler file tree. It would be much better if this is also possible for a local list folder.

ww7 commented 2 years ago

I agreed, currently favorite folder replace already opened folders, this not useful and annoying Would be great to have: 1) option in context menu to open folder in new window 2) on favor folder click: a) if folder already added to workspace - reveal in explorer sidebar b) if folder not in tree of workspace folders - add folder as "workspace folder"

oleg-shilo commented 2 years ago

BTW, the option is already there:
The latest release (at least) already have "do nothing" approach implemented for already opened. Though it is applicable for the scenario:
```
folder_a
    folder_aa
    folder_ab
```
Thus
```
if (clicked_folder == folder_a && open_folder == folder_a)
    do_nothing
else
    open(clicked_folder)
```
And this will of will open the clicked folder folder_ab if already opened folded is folder_a. This is what we want to change. Do I understand the proposal correctly?

oleg-shilo commented 2 years ago

Assumed the "yes" answer.

Done. Please test the pre-release: v1.5.9

tjyuyao commented 2 years ago

With v1.5.9, I am still facing the following case:

if (clicked_folder == folder_a && open_folder == folder_a)
    open(clicked_folder)

When I click "Open in new window", however, it reveals the folder in the file tree.

I am not sure where the problem is. Would you mind checking the modification again?

oleg-shilo commented 2 years ago

Ah... sorry I missed the point with open in new window Good I did not want to release it just yet

oleg-shilo commented 2 years ago

Would you mind testing this one please: favorites-1.5.10.vsix.zip

Keep in mind VSCode (as for v1.63.2) have these anomalies/peculiarities of vscode.openFolder and vscode.open

vscode.open opens files OK but throws the exception on an attempt to open afolder
vscode.openFolder opens files and folders OK
vscode.open does not have the option to open a file in a new window vscode.openFolder does
vscode.openFolder ignores the call if the folder (or workspace) is already open either in the current window or a new one

tjyuyao commented 2 years ago

With 1.5.10, the open in new window works as expected, and once a subfolder is opened in a new window, clicking on that folder will activate the corresponding new window.

But if this subfolder is not opened in a new window, clicking on the folder icon will open this subfolder in the current window as root working directory, instead of "do nothing" or "reveal in side bar".

My VSCode version info:

Version: 1.63.2
Commit: 899d46d82c4c95423fb7e10e68eba52050e30ba3
Date: 2021-12-15T09:39:46.686Z
Electron: 13.5.2
Chromium: 91.0.4472.164
Node.js: 14.16.0
V8: 9.1.269.39-electron.0
OS: Linux x64 5.4.0-77-generic

ww7 commented 2 years ago

With adding subfolder only root folder added instead subfolder Also opening folder works as opening instead all already opened (as root folder)

oleg-shilo commented 2 years ago

@ww7, Serhii, sorry, it looks like it was a typo in your post and the meaning got distorted.

@tjyuyao, I am not sure we are on the same page.

Below are the requirements

if clickedfolder == vscode.openedFolder
    reveal(vscode.openedFolder) // vscode will switch explorer and try to reveal
else if clickedfolder.isSubfolderOf(vscode.openedFolder)
    reveal(vscode.clickedfolder) // vscode will switch explorer and try to reveal
else
    if request.openInNewWindow
        vscode.openInNewWindow(clickedfolder)
    else
        vscode.open(clickedfolder)

v1.5.11 (I am preparing it). meets them. I expect the experimental v1.5.10 to do too but it makes sense to start stressing v1.5.11 as it had a few significant changes regarding opening items - support for remote connections (#31).

I'll let Serhii to elaborate and if it does not reveal any new geps will share with you guys the new build.

tjyuyao commented 2 years ago

Currently the part of my observation that doesn't meet my expectation is that a clicking of a subfolder will substitute the origin vscode.opendFolder and become the new root opened folder. Does this reproduce in your environment? If so, my reason for disliking this behavior is that the original root folder is closed and I still need it. I kind of realize that whether the user still needs the root folder is not known, so maybe can we have a new context-menu entry for one of the options: reveal in sidebar or open in current window?

if clickedfolder == vscode.openedFolder
    reveal(vscode.openedFolder) // vscode will switch explorer and try to reveal
else if clickedfolder.isSubfolderOf(vscode.openedFolder)
    reveal(vscode.clickedfolder) // vscode will switch explorer and try to reveal <---- here
else
    if request.openInNewWindow
        vscode.openInNewWindow(clickedfolder)
    else
        vscode.open(clickedfolder)

oleg-shilo commented 2 years ago

Yes. Correct. This is what I implemented interpreting your request as "it is what you want". You indicated that "nothing" is acceptable but I understood that you would prefer the "reveal" option.

Well, if it was not the case then I am happy to roll it back to "do nothing".

Agree?

In fact, we can do it as a configurable option either:

open clicked subfolder as it does currently (default)
"do nothing" if the parent folder is opened and pops the error message

tjyuyao commented 2 years ago

Yes, I would prefer the "reveal" option.

My idea is that opening clicked subfolder is the correct choice for a global favorite list, but not for a local one. I see the global list as a shortcut to start new workspaces, while the local list as a navigator/shortcut list for opening files/revealing folders in a very complicated already opened workspace. I want to access the favorite items in the scope of the currently opened folder that may have too complicated hierarch of subfolders, so clicking a subfolder should respond similar to clicking a file. Do I misunderstand the purpose of inventing .fav/local.list.txt?

Sorry I didn't have the experience of using Favorites as a global navigator before. This may be the root of my different expectation than yours.

Thanks for your work anyway. I appreciate it very much!

oleg-shilo commented 2 years ago

Huang, there is a problem with the user experience of your proposed solution.

In the explorer view, users cannot easily distinguish between global and local lists. Thus it would create a questionable experience when the user clicks "open" and sometimes it acts and sometimes it does not.

Reveal also does not work very well either. When the item you are revealing is not opened already (e.g. reveal a subfolder of the already opened folder) VSCode simply switches to the explorer and stays there without selecting the folder.

All these considerations made me make this decision.

There will be a configurable option:

if clickedfolder == vscode.openedFolder
    reveal(vscode.openedFolder) // vscode will switch explorer and try to reveal

else if clickedfolder.isSubfolderOf(vscode.openedFolder)

    if settings.doNotOpenFoderIfItsParentAlreadyOpened
        showMessage("parent folder is already opened") // effectively "do nothing"
    else
        vscode.open(clickedfolder)

else

    if request.openInNewWindow
        vscode.openInNewWindow(clickedfolder)
    else
        vscode.open(clickedfolder)

Give me a day and I will make the release

oleg-shilo commented 2 years ago

OK. Done: https://github.com/oleg-shilo/Favorites.vscode/releases/tag/v1.5.11

Added two new settings to control the behaviour.

Ironically, I found that I like the most switching subfolders off completely. It is my own setting now :) Thank you all for a great feature suggestion.

I will publish it on VSCode marketplace tmr

ww7 commented 2 years ago

Not sure why, I can't add subfolders in Favorites list, error: The active document is already in the Favorites.

oleg-shilo commented 2 years ago

Not sure what exactly you are referring to by subfolders cannot be "added".

What folder if any is opened. What are the folders in the current favorites list. Basically can you describe the test case please.

ww7 commented 2 years ago

I have opened folder from Favorites list, so root folder already in list. In File Explorer selected any folder (subfolder accordingly to root folder) and clicked Favorites button for add active folder Is it designed to add only root folder?

oleg-shilo commented 2 years ago

Ah, I see. "Add active folder" adds" the folder that is open in IDE. Thus in your case indeed you are trying top add the folder that is already in the list.

What you can do is either

open the desired folder in VSCode and click "Add active workspace/folder..."
Or, at any time, just "Manage Favorites" and simply add the path to the desired folder as a text:

ww7 commented 2 years ago

@oleg-shilo Thank you, I know. I think correct behavior is a adding of selected folder. Seems this is also not possible as revealing Favorites folder.

oleg-shilo commented 2 years ago

Serhii, actually, it is technically possible. :)

"Revealing" is a different kettle of fish. Revealing is problematic as you may have nothing to revile if the item you are trying to reveal is not present in the explorer'.

Acting on the selected item is possible but so far the operations for selected items are limited to "move up/down" and remove. And only for the items, that are in the list already (root items). Thus adding another context menu item ("Add to favorites") is possible but it will need to be done only for the items that are not in the list. Very difficult considering management of the context menu in VS Code needs to be done from the package.json but not from the extension code at runtime.

Sorry, that's why it is not an option.

oleg-shilo / Favorites.vscode

folders in `.dav/local.list.txt` should not be opened in a new window #29