micro icon indicating copy to clipboard operation
micro copied to clipboard

call `onSetActive` consistently whenever a bufpane receives focus

Open matthias314 opened this issue 1 year ago • 3 comments

I find the current behavior of onSetActive quite strange (as remarked in #3563 already):

  • When you start micro, onSetActive is not called. This also applies if you list one or more files on the command line that are opened in separate tabs or splits.
  • When you open a new tab or switch between tabs, onSetActive is called. When you close a tab (that is not the only tab), then most of the time onSetActive is called. However, if you open several files on startup and then close one tab after the other, onSetActive is never called.
  • When you open a new split, onSetActive is not called. When you switch between splits or close a split, onSetActive is called.
  • On top of that, if you open a "raw" buffer, then onSetActive is called with nil as argument.

This PR changes the behavior as follows: "active" is interpreted as the bufpane that can receive user input. Hence there is exactly one active bufpane at any time (or none if the "raw" buffer is open). Whenever a bufpane becomes active, onSetActive is called. This may happen through opening, switching or closing tabs or splits and also at startup. Th call for a "raw" buffer is eliminated, and the double mentioning of onSetActive in the documentation is corrected.

I've used this patch for some time now and it seems to work well. Nevertheless, please have a look at the changes. Maybe there are better ways to achieve the goal. Or maybe you want onSetActive to behave differently. I myself find if difficult at present to use onSetActive in a meaningful way.

matthias314 avatar Dec 20 '24 15:12 matthias314

Regadreless of onSetActive I like the changes around vsplit/hsplit i.e. making BufWindow inactive by default and then calling setactive explictly on VSplitBuf/HSplitBuf sounds good.

Your changes to TabList.SetActive affect more than just the onSetActive behavior, since they explicitly set something like "oldTab".CurPane().setActive = false during a tab change. Currently, in Micro, every inactive tab still has a pane with active = true in its struct. Your change alters this by making CurTab.CurPane() the only pane with active = true. Is this change intentional or desirable?

Also, explicitly calling BufPane.SetActive during a tab change modifies a bit the gutter message behavior, seems like an improvement.

However, if you open several files on startup and then close one tab after the other, onSetActive is never called.

From what I can see whenever you close a tab that isn't the last one onSetActive won't be called. This happens because (t *TabList).RemoveTab doesn't call (t *TabList).SetActive, as it's unnecessary. The active tab is managed by TabList.TabWindow.active, which simply holds the relative index of the active tab within TabList.List, i.e. if your active tab is the first one and you close it, the active index will still point to the first position.

On ForceQuit() calling MainTab().CurPane().SetActive(true) next to RemoveTab() looks odd. What if I call RemoveTab() on lua?

I wonder if a naive approach like storing MainTab().CurPane() on every action and checking it against the previous iteration would be a bad or dumb idea?

cutelisp avatar Jul 23 '25 03:07 cutelisp

Thanks a lot for the feedback.

Your change alters this by making CurTab.CurPane() the only pane with active = true. Is this change intentional or desirable?

It's at least intentional. Without it, onSetActive would not be called when switching tabs.

What if I call RemoveTab() on lua?

Good point. It would probably be better to move MainTab().CurPane().SetActive(true) inside RemoveTab().

I wonder if a naive approach like storing MainTab().CurPane() on every action and checking it against the previous iteration would be a bad or dumb idea?

I'm open to other solutions. My motivation was to change the behavior of onSetActive to something that I find useful. As I explained in the OP, that's not the case at present.

It would be good to hear from the maintainers if they are interested at all in modifying onSetActive. If not, then we don't need to discuss this further.

matthias314 avatar Jul 23 '25 23:07 matthias314

It would be good to hear from the maintainers if they are interested at all in modifying onSetActive. If not, then we don't need to discuss this further.

Sounds fair

cutelisp avatar Jul 24 '25 01:07 cutelisp