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

Notebook menu: Add submenu to close all documents in/not in current folder #2346

Open
wants to merge 6 commits into
base: master
from

Conversation

@ntrel
Copy link
Member

commented Oct 8, 2019

The first menu item closes all documents in the same folder as the notebook tab that was clicked.
The second menu item closes all documents not in the same folder as the notebook tab.

See #2346 (comment) for rationale.

Also fixes iterating docs for Close other Documents in notebook tab order [moved to separate PR].

I'd like to add a folder menu item for Open in New Window too.

Screenshot:
image

continue;
if (other_folders && g_str_has_prefix(doc->real_path, dir))
continue;
if (!other_folders && !g_str_has_prefix(doc->real_path, dir))

This comment has been minimized.

Copy link
@ntrel

ntrel Oct 8, 2019

Author Member

Checking for a prefix means subdirectories will also be matched, not sure whether that's good or bad.

This comment has been minimized.

Copy link
@elextr

elextr Oct 8, 2019

Member

If you mean closing in the whole tree subtree, that probably would be better on the documents sidebar where the specific root directory can be selected, you may not have files in that specific directory open, only subdirectories.

This comment has been minimized.

Copy link
@ntrel

ntrel Oct 9, 2019

Author Member

you may not have files in that specific directory open, only subdirectories.

Sorry, I'm not following this part of your comment.

This comment has been minimized.

Copy link
@ntrel

ntrel Oct 9, 2019

Author Member

There has to be at least one file with that full directory because there wouldn't be a notebook tab otherwise.

@ntrel ntrel force-pushed the ntrel:folder-menu branch from 55f7189 to e2a098a Oct 8, 2019
@elextr

This comment has been minimized.

Copy link
Member

commented Oct 8, 2019

Could be useful at times, quick looks ok, havn't tested.

src/notebook.c Outdated Show resolved Hide resolved
src/sidebar.c Outdated Show resolved Hide resolved
@kugel-

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

sidebar tree #1813 implements the "in folder part" through the side bar, i.e. when you close a folder (via popup menu or middle mouse click) then all files below that folder will be closed. The other use case could probably also be implemented there. Do you think it makes this PR obsolete?

I never use the notebook popup menu, mainly because I always have many files open so I always have to scroll.

@elextr

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

I never use the notebook popup menu, mainly because I always have many files open so I always have to scroll.

ditto because I have the tabs turned off

ntrel added 2 commits Oct 9, 2019
@ntrel

This comment has been minimized.

Copy link
Member Author

commented Oct 9, 2019

#1813 implements the "in folder part" through the side bar

Actually the sidebar 'Show Paths' option already supports closing all docs in a folder.

The other use case could probably also be implemented there. Do you think it makes this PR obsolete?

I think this pull makes the feature more discoverable, a user might not think to try right clicking on a folder in the sidebar documents. This pull also closes docs in notebook tab order instead of filename order.

Also, the notebook popup already has Close All, Close other documents and Close tabs to the right. I think this fits in with those commands.

@kugel-

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

Okay, your call.

 Actually the sidebar 'Show Paths' option already supports closing all docs in a folder.

It doesn't do it recursively because each sub-(sub-) directory gets a top-level node just below the root.

@ntrel

This comment has been minimized.

Copy link
Member Author

commented Oct 9, 2019

It doesn't do it recursively

True, I'm not sure whether this pull should or not. (Recursive closing does make sense for your sidebar tree PR).

@codebrainz

This comment has been minimized.

Copy link
Member

commented Oct 10, 2019

I do use the tab menu a lot, but I'm not a big fan of the UI of this PR. Even before, I thought there were too many "close" options in the menu, and this adds another. My inclination would be too put them all in a submenu, but while it de-clutters the menu, it makes them more annoying to use. Also I don't like how the new item is grouped with Open in New Window rather than at the bottom of the existing "close" related items, in the same section/group as them.

I suppose I'm biased though since I have never had a need for this feature and it will only add more stuff into an already (usually) massive menu, negatively affecting my personal usage.

For example, with maybe around half of Geany's C files open, I already have to scroll to access the menu items I do use:

geany_tab_menu

This pull also closes docs in notebook tab order instead of filename order.

Isn't that what #2347 is for?

@codebrainz

This comment has been minimized.

Copy link
Member

commented Oct 10, 2019

Ah, I'm reading my emails in chronological order, hadn't seen the UI issues are already being discussed/addressed in a separate PR #2348.

@ntrel

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2019

I thought there were too many "close" options in the menu, and this adds another. My inclination would be too put them all in a submenu, but while it de-clutters the menu, it makes them more annoying to use

I think putting the multiple document close icons in a submenu would be good UI design, so people don't accidentally click them. They should be harder to use.

@ntrel

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2019

Isn't that what #2347 is for?

That pull is for the existing close other documents command. I was pointing out the close order difference of this pull vs the sidebar documents close folder. Perhaps the latter should close in tab order too rather than alphabetic order?

@ntrel

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2019

I have never had a need for this feature

Maybe you keep all files in a project open always. But as you mention, this makes it harder to switch to the document you need. It is also impractical for slower systems as it slows down opening a project and uses more memory.

I would use this e.g. when I've finished looking at files in the ctags subfolder. It's also useful when I've opened a folder in a current project and would rather it was in a separate project. Currently I save the project and copy the project file. Then I close the folder(s) I don't need in each project.

Once I implement Open in New Instance for this pull's Documents in Folder menu, it will make splitting a project a bit easier.

@ntrel

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2019

it will only add more stuff into an already (usually) massive menu

It only adds one menu item, (and the multiple document close items should be put in a submenu as I mentioned).

@ntrel

This comment has been minimized.

Copy link
Member Author

commented Oct 11, 2019

Once I implement Open in New Instance

See #2352.

@codebrainz

This comment has been minimized.

Copy link
Member

commented Oct 12, 2019

I think putting the multiple document close icons in a submenu would be good UI design, so people don't accidentally click them. They should be harder to use.

I think the opposite, when possible/not too big, IMO it's better to keep a flat menu structure so it's easier to use and requires less clicks (and less mnemonics if you use accelerators). It's not like closing documents is dangerous and presumably it's rare even with a lousy trackpad or muscle control.

A nice thing about #2348 is that it alleviates my main concern that the menu was too crowded and allows stuff like this PR to get rid of the submenu and also adding items like "close to the left" or "open in split window" or whatever stuff we want.

I was pointing out the close order difference of this pull vs the sidebar documents close folder. Perhaps the latter should close in tab order too rather than alphabetic order?

IMO, the reasoning in #2347 is sound and should apply to any operation on multiple tabs (closing, searching, etc.) no matter which feature triggers the action.

Maybe you keep all files in a project open always.

My usual workflow is to keep maybe 1-15 tabs open, and close them when I don't need them for the time being (often using one of the multi-close menu items in the tab menu). I usually open documents using the ProjectOrganizer treeview, with one or two folders expanded, so there's no reason for me to keep loads of files open, every file in the project is a double-click away. It helps that I have a 4k monitor without scaling, so I can see lots of files at a glance in the sidebar.

It only adds one menu item, (and the multiple document close items should be put in a submenu as I mentioned).

If #2348 gets merged, I have no objections to this PR whatsoever, and even without, I don't feel strongly against this PR, I just most likely won't use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.