spyder-ide / spyder

Official repository for Spyder - The Scientific Python Development Environment
https://www.spyder-ide.org
MIT License
8.1k stars 1.57k forks source link

Implement a full UI for the Bookmarks feature #9021

Open CAM-Gerlach opened 5 years ago

CAM-Gerlach commented 5 years ago

PR #8481 implements a bookmarks feature for Spyder's Editor. Here, we will discuss and decide on an appropriate UI for the same.

@ccordoba12 's relevant comments:

For the UI part I'd like to see:

  1. An icon next to the line that has a bookmark.
  2. A panel to browse bookmarks.
  3. A keyboard shortcut to move to the next/previous bookmark.

We can add the list of bookmarks to the file switcher or add a shortcut to move to the bookmarks pane and filter file names with the keyboard there.

@CAM-Gerlach 's proposed UI:

CAM-Gerlach commented 5 years ago

In summary, while your alternative suggestions would be valuable in themselves (particularly a Ctrl-Tab-like shortcut to recall recent bookmarks), they don't replace the fast, random, and consistent access to specific favorite bookmarks that directly assignable shortcuts provide.

However, we still need some form of keyboard shortcut UI to access them, as the whole point is quick access and instantly tabbing back and forth between

To address this, we can add the list of bookmarks to the file switcher or add a shortcut to move to the bookmarks pane and filter file names with the keyboard there.

We could potentially have an option to include bookmarks in Ctrl-Tab, to quickly toggle between the most recent ones, or (to avoid cluttering it) a dedicated shortcut that invokes similar functionality with just bookmarks (basically your previous/next idea with the order being MRU). Making them work within the Ctrl-P switcher or be quickly accessed within the bookmarks pane by keyboard without shortcuts requires work on both our and the user's end to label bookmarks, and the latter also potentially disrupts the user's pane layout by calling up the bookmarks pane (unless we make a separate bookmark switcher like the Ctrl-P one, or just use the it directly).

It won't be the primary UI. We agreed on the panel too

Sorry, I meant the primary keyboard UI. We agree on the panel, but unless the user manually labelled all shortcuts to allow KB-based searching (discussed above), the time/effort to open the panel, navigate to it with the mouse, scroll through to find the desired shortcut, click it, and switch back to the Editor is likely not much of a savings for frequently-accessed bookmarks compared to simply navigating to the location the conventional way, particularly considering the complexity cost.

MRU-order previous/next shortcuts provide a lot of unique benefit, but they don't replace the need for direct KB shortcuts to make the system efficient for non-sequential access to a consistent set of file locations, which is important for a lot of workflows and is a key advantage of our bookmark system as implemented.

ccordoba12 commented 5 years ago

I'll come back to this after we release beta2. I don't have time for it right now, but thanks for input.

oscargus commented 4 years ago

I started playing around with this a bit before finding this issue (and probably didn't read up well on previous discussions either...). Here's what I got so far:

image

Basically, the bookmarks are added by clicking on the additional column panel. There is support for multiple bookmarks per line, but only because it was like that in #8481. Now, the multiple bookmark icon is a result of bad coding, but shows what it may look like (probably better scaled, there is a bookmark-multiple icon in Material Design, but it has not propagated to QtAwesome yet).

Ideally, there should be numbers in the bookmark icons, see e.g. TeXStudio:

image

My idea was to add a pane with the bookmarks (covering only a single file, similar to #9483

One problem I see (feature I do not find useful) is that one can set multiple bookmarks per line. This complicates things a bit. Graphically, it doesn't really make sense to click and set at the current cursor position. Would be much easier to just have a line bookmark.

I'll put this on hold for now, but given line bookmarks (+ columns), the graphical interface (panel + pane) should be rather straightforward to finalize. (Removing the column support is probably the most time consuming part...)

No strong opinions on keyboard shortcuts, but it would make sense to be able to at least jump to the bookmark from the keyboard in a reasonably efficient manner. I (think) I prefer jumping to a given bookmark rather than previous/next, but I rarely use it (although see the potential benefit).

ccordoba12 commented 4 years ago

Basically, the bookmarks are added by clicking on the additional column panel

Do we need a new column panel for this?

There is support for multiple bookmarks per line, but only because it was like that in #8481

This makes no sense to me (and I was not aware PR #8481 added that), so you can safely remove it.

Ideally, there should be numbers in the bookmark icons, see e.g. TeXStudio

This doesn't seem too important to me.

My idea was to add a pane with the bookmarks

Yes, that's the general plan.

No strong opinions on keyboard shortcuts

This should be shown in the bookmarks pane, otherwise it doesn't make sense to me. @goanpeca, do you have thoughts about this?

goanpeca commented 4 years ago

This should be shown in the bookmarks pane,

You mean as buttons @ccordoba12?

ccordoba12 commented 4 years ago

Next to the bookmark position, if we show bookmarks in a table, I'd say.

ccordoba12 commented 4 years ago

And as text, so that users are aware of the shortcut associated with the bookmark.

goanpeca commented 4 years ago

Yes, I agree with this.

goanpeca commented 4 years ago

Yes, I agree with this.

oscargus commented 4 years ago

Thanks for the feedback! I'll see what I can do. Good that qtawesome was updated with new fonts (I assume including the "missing" one).

Basically, the bookmarks are added by clicking on the additional column panel

Do we need a new column panel for this?

Not sure. Probably to show it, but if they are keyboard only (with a pane), I guess not. But if it is made configurable, I guess that it should be fine to have both option?

There is support for multiple bookmarks per line, but only because it was like that in #8481

This makes no sense to me (and I was not aware PR #8481 added that), so you can safely remove it.

Good to hear! What about multiple bookmarks to the same line? Should they just be added or should they be overwritten? I guess it is probably not that important as in most cases they should have been added by mistake, but good to know what the preferred way of handling it is.

Ideally, there should be numbers in the bookmark icons, see e.g. TeXStudio

This doesn't seem too important to me.

OK! Then I'll wait with it for (possibly) later. I noted that there are several other icons with numbers in the recent Material Design Icons (? might have been some of the other qtawesome ones), but nothing that looks like bookmarks (squares and circles primarily).

My idea was to add a pane with the bookmarks

Yes, that's the general plan.

Good!

No strong opinions on keyboard shortcuts

This should be shown in the bookmarks pane, otherwise it doesn't make sense to me. @goanpeca, do you have thoughts about this?

Ctrl+shift+number to set bookmarks and ctrl+alt+shift+number to restore them? Which of course could enable ctrl+shift+alt+(+/-) to go to previous/next and ctrl+shift+(+/-) to add next/delete current. One may also consider that new bookmarks beyond 10 are added, but only accessible using previous/next (and pane/panel), but no keyboard shortcuts.

Should these go into the keyboard shortcuts? From some perspective they should, but then there will be 24 additional out of at least the ones for adding/removing will be rather repetitive. Problem is then if someone already has reassigned, say, ctrl+shift+number. Probably the go to next/previous and add next/remove keyboard shortcuts should be there at least.

agentmorris commented 4 years ago

Weighing in here with UI opinions, apologies for some duplication of a comment I made on 8481 (before I knew this PR was the right place to discuss the UI).

My opinion should be given very limited weight here, because I don't have the familiarity w/Spyder to implement anything. Nonetheless, weighing in with a vote for a UI more or less identical to the very simple approach that both Matlab and Visual Studio take, namely:

These are Matlab shortcuts; the keyboard shortcuts are different in VS, but the paradigm is identical.

There is no numbering to bookmarks. AFAIK bookmarks are not saved in either Matlab or VS.

I personally prefer this simple approach to vi-style named/numbered bookmarks; the way I use this (and apparently the way most Matlab users use this, else they presumably would have changed it) is pretty temporary (but important!), and the cognitive load of remembering multiple bookmark numbers would be too much.

This does not serve the purpose of true persistent favorites that some comments have expressed a preference for above.

Excited to see any form of this feature happening!

DavidCohen2 commented 2 years ago

First of all, thanks to all developers - spyder today is a really great editor - thanks.

I searched for the bookmarks feature and I understand that this feature is not implemented yet. Is there is any date for that feature release - it's annoying to scroll back and forth... I'm using spyder 5.1.5

Again thanks to you all.

ccordoba12 commented 2 years ago

Hey @Davidcohen2, right now we only have the backend for this feature but not an interface for it, sorry. We'll try to add it in a future release.

DavidCohen2 commented 2 years ago

Thanks for your response.

As I saw, in one of the posts - the conditional breakpoint along with the breakpoints panel, could be used as a workaround. The condition also could be informative like "main function call"==1.