Why are there no getters for tabs?

[matthiasbeyer]

Hi,

as far as I can see, there are no getters for the current (or any other) tab in the interface, is that right?

Why not?

[fin-ger]

According to the rust naming conventions we have active_tab() and set_active_tab() both implemented on TabView and TabPanel. A get_active_tab() would therefore violate this naming convention. However, I think you mean an ambiguity in our API design where active_tab() is not providing the tab itself but the identifying key.

First of all, the choice behind this was that users can use a NamedView with the Nameable trait to access a tabs view (by simply wrapping a tab with a NamedView). We wanted to avoid yet-another-identifying mechanism in cursive (there also was the whole id vs. name discussion in cursive itself). That's also why we are returning a key for the current tab and not the view. Also, the key is used to reorder, remove, swap tabs, etc. It seemed reasonable to stick to the 'a tab is just a key' approach (remember a key can easily be the name of the tab by choosing a string as the tab's key like in our examples).

However, this adds some unexpected behaviour to our API as it is not straightforward to get the View of a tab. We wanted to avoid to force each tab to be wrapped with a NamedView and also let the user decide on the names. However, redesigning the API to match a key to the tab's name might be a better approach from a user's perspective. This would add some restrictions to the key data type, as it must fit into the type constraints of a NamedView (which I think was just a String). This means no u8 or similar will be possible as tab identifiers. But I think this should have no performance impact on our TabPanel. There might be little performance penalties by forcing each tab to be wrapped in a NamedView but I think this would be ok.

@jwuensche What are your thoughts on this?

[matthiasbeyer]

The thing with the nameing-views approach is that the user has to keep track of the names, which is a burden.

If I have something like a commandline in my app (think vim), and the command executes on the currently visible tab, I have to look up what is the right one and then fetch that from cursive. With a simple getter, this is a no-brainer.

The same btw for the cursive_multiplex crate, where I'm preparing a similar patch right now.

[fin-ger]

Regarding the naming active_view() would suffice. So a tab would be an ID and the view is a View.

Let's wait for Johannes to give some input on the yet-another-identifying mechanism problem. He wrote most of the parts of cursive-tabs and cursive-multiplex.

[matthiasbeyer]

Maybe some more narrative for the whole thing:

I'm working (on and off to be honest) on a MUA für TUI with notmuch as backend in Rust, and stumbled across the issue of not having getters in this crate (and cursive_multiplex) while rewriting the abstractions (see here, does not yet compile because of different cursive versions from different dependencies) for my program. The idea is to have a view that has tabs, where each tab has a multiplexer and each view in the multiplexer can be anything, eg.

• lists of mails (output from notmuch queries)
• a single mail, shown
• a texteditor to craft a new mail (not there yet, might be embedded vim later... if I get there eventually)
• whatever else.

Maybe there's a better/cleaner way to do that, but right now I cannot see it. Using named views might be an option, but as said, keeping track of the names is IMO not as trivial as one things. For example, if I hit :edit in my MUA (not implemented, but stay with me for the sake of it), the command would be executed on the currently visible tab, on the current active multiplexer split, in the view that is there. Keeping track of which tab and which mux is currently selected is hard. Especially when giving multiple options to jump around.

I hope that reinforces my point.

---

Maybe we can, in the progress of the MUA, outsource the tab+mux part into a common crate (with a "call commands" helper for conveniently calling command like in vim and implementing keybindings), at some point.

[fia0]

Hi @matthiasbeyer, I can totally see your point with your remark, especially in situations with a lot of tabs and multiplexers this can get really annoying to keep track of. In general it's a good idea to add an active_view to fetch the currently focused view. Think your PR is a good integration of this and I would like to include it.

Furthermore, with the whole yet-another-identifier thing, had a short chat with Fin about it and since we are practically holding just another identifier on top of cursive which is a bit of an anti-pattern. (I think most people have chosen a String as identifier anyway) We want to present here a small redesign of how the API works:

1. Drop our own identifier and use String as in cursive.
2. Change the type of views we hold to NamedViews as proposed by Fin. This is the most cursive way to integrate tagged views we hold in our the tab view.
3. Again for clarity, I suggest that we change add_tab to take NamedView and let the user wrap the views for themselves. This would require changes in the user code but I think is still worth the effort, as wrapping views for the user with the current interface is again less clear. Most usage can be migrated like this:

   tabs.add_tab("some fancy id", MyView::new());
   // to
   tabs.add_tab(MyView::new().with_name("some fancy id"));

4. Next, to active_view() we might want to add a tab_view(name) or something similar to get the view of a tab currently not in focus. Though this is just an idea and the general find_name might suffice here, although a deep search is performed in this case.

[matthiasbeyer]

Sounds like a good way to go!

[fia0]

I opened a new branch rework/NamedView a few days ago to implement this proposal. The changes brought the added benefit that with the general rework, the API could be cleaned up a bit, whereas some methods for removing required values and others references. Additionally, a specific error type IdNotFound has been added to get rid of the ominous Result<(),()> return we had for some methods. Will add the commits to main now and see if everything works as expected and then continue with your PR :smile: