counsel-find-file: Support opening in other tab

[Hugo-Heagren]

I've been learning recently about tab-bar-mode, which is great. Like opening in the other window, or other frame, I think it would be useful for functions like counsel-find-file to have an action for opening in the other tab. I think t is currently free in the default actions list, and it would make sense for this. I would be happy to write up a PR if this seems like a good idea.

[basil-conto]

SGTM, thanks!

[Hugo-Heagren]

Here's a pull request which adds other-tab actions on t to most commands for which I think they are relevant. Happy to work on any more that you can think of and add them to the commit though?

The only command I haven't done is counsel-bookmark. To continue in the same coding style as the others, this would need a function bookmark-jump-other-tab, which doesn't exist yet. This seems more like something which should be implemented in the emacs code base (in files.el, along with e.g. bookmark-jump-other-frame) and then used by ivy, so I'm hesitant to just write it myself and use it here. I'll have a look at submitting a patch to emacs. Honestly I'm sure I'm one only very few people who would use it anyway, so it's not really too great a problem.

[basil-conto]

Thanks!

To continue in the same coding style as the others

No need to worry about that. If you prefer to have such a command now, there's no need to wait for Emacs to acquire it as well. If and when Emacs acquires it, we can always adapt Counsel accordingly.

this would need a function bookmark-jump-other-tab, which doesn't exist yet

FWIW, my impression is that vanilla Emacs nowadays prioritises the more general idiom of other-window-prefix (C-x 4 4), other-frame-prefix (C-x 5 5), and other-tab-prefix (C-x t t) over individual *-other-window, *-other-frame, and *-other-tab command variations, respectively.

[basil-conto]

Native tabs are new in Emacs 27, but Ivy supports Emacs 24.5+. Should the addition of these "other tab" actions be conditional on the fboundp/functionp presence of the required underlying functions?

[Hugo-Heagren]

Native tabs are new in Emacs 27, but Ivy supports Emacs 24.5+. Should the addition of these "other tab" actions be conditional on the fboundp/functionp presence of the required underlying functions?

Yes, that sounds sensible to me. I very briefly had a look at ways of doing this. The simple way is to just use ivy-add-actions inside a conditional which checks for compatibility. This would work fine, but then the ivy-add-actions would run after the initial ivy-set-actions which established the original list, so whenever the tab actions are added, then they are added at the end. This is a small thing, but it's nice to get stuff like this right if you can--I originally put the "other tab" action just after the "other window" and "other frame" actions, because that seemed to make sense. Putting it at the end seems wrong.

So to enforce the right position, I tried

(ivy-set-actions
 'counsel-find-file
 `(("j" find-file-other-window "other window")
   ("f" find-file-other-frame "other frame")
   ,(when (fboundp 'find-file-other-tab)
       '("t" find-file-other-tab "other tab"))
   ("b" counsel-find-file-cd-bookmark-action "cd bookmark")
   ("x" counsel-find-file-extern "open externally")
   ("r" counsel-find-file-as-root "open as root")
   ("R" find-file-read-only "read only")
   ("l" find-file-literally "open literally")
   ("k" counsel-find-file-delete "delete")
   ("c" counsel-find-file-copy "copy file")
   ("m" counsel-find-file-move "move or rename")
   ("d" counsel-find-file-mkdir-action "mkdir")))

This works fine if the test passes, but if it doesn't then the whole sexp ((when (fboundp 'find-file-other-tab) ("t" find-file-other-tab "other tab"))) returns nil, which then becomes an element of the action list for the command. Ivy doesn't recognise nil as an action, so complains when running ivy-dispatch.

There are a few solutions:

• Ignore positioning. It doesn't matter that much where the actions appear in the list. I don't support this, but I can see why it might not be worth writing extra code just to make things 'look right'.
• Write an api function for adding actions at a certain place in the actions list, a bit like the way transient-insert-suffix works.
• Make ivy ignore nil entries in the actions list. I don't know how difficult this would be to code, but if it's feasible I think this might be quite cool. It would allow complex logic around whether an action shows up, or what it does when present. In the current case, some actions are only available if contextual system conditions are met, but there could also be conditions on the candidate itself: certain actions only apply to certain candidates within the same ivy session, so they only show up in those candidates dispatch menus. This could be pretty useful.

What do you think?

EDIT: turns out it's pretty easy to code, at least I think it is. Here is my Very Simple amateur-elisp solution, just filtering out all the nil-evaluating values. Worked for me:

(defun ivy-read-action ()
  "Change the action to one of the available ones.

Return nil for `minibuffer-keyboard-quit' or wrong key during the
selection, non-nil otherwise."
  (interactive)
  (let ((actions (cl-remove-if 'byte-compile-nilconstp
                               (ivy-state-action ivy-last))))
    (if (not (ivy--actionp actions))
        t
      (let ((ivy--directory ivy--directory))
        (funcall ivy-read-action-function actions)))))

[basil-conto]

This works fine if the test passes, but if it doesn't then the whole sexp [...] returns nil

If I understood correctly, I think you just need to splice, rather than unquote, the conditional part into the rest of the list, right? E.g.:

(ivy-set-actions
 'counsel-find-file
 `(("j" find-file-other-window "other window")
   ("f" find-file-other-frame "other frame")
   ,@(and (fboundp 'find-file-other-tab)
          '(("t" find-file-other-tab "other tab")))
   ("b" counsel-find-file-cd-bookmark-action "cd bookmark")
   ("x" counsel-find-file-extern "open externally")
   ("r" counsel-find-file-as-root "open as root")
   ("R" find-file-read-only "read only")
   ("l" find-file-literally "open literally")
   ("k" counsel-find-file-delete "delete")
   ("c" counsel-find-file-copy "copy file")
   ("m" counsel-find-file-move "move or rename")
   ("d" counsel-find-file-mkdir-action "mkdir")))

[Hugo-Heagren]

If I understood correctly, I think you just need to splice, rather than unquote, the conditional part into the rest of the list, right?

Thanks! I didn't know about the @ notation for splicing, haven't quite got round to reading the elisp manual yet. I've used that, and also added support for counsel-bookmark.

[basil-conto]

Thanks!

I didn't know about the @ notation for splicing, haven't quite got round to reading the elisp manual yet.

If you're still interested, the relevant docs are under C-h v ` RET and (info "(elisp) Backquote").

BTW, I think this PR is just large enough that I cannot merge it without a copyright assignment (CA) to the FSF. Do you already have a CA on file? If not, would you be willing to submit one?

If you're unfamiliar with this, see:

• (info "(emacs) Copyright Assignment")
• https://github.com/abo-abo/swiper/blob/master/CONTRIBUTING.org#copyright-assignment

[Hugo-Heagren]

Do you already have a CA on file?

Not yet.

If not, would you be willing to submit one?

Happy to. I'm a student at a fairly well know university, so I might want to check with them first, but I can't them kicking up a fuss. I'll get on it.

[basil-conto]

Happy to.

Great!

I'll get on it.

Thanks, just ping back here when the process is complete, or if you have any questions.

[Hugo-Heagren]

or if you have any questions

So, I sent off the initial email to the FSF. I got a response with a form with a section for me and one for an FSF referee of some kind. I filled out my part (leaving the other blank) and absent any other instructions, emailed back to the person who sent it to me. Should I expect some sort of confirmation of my CA, or am I good to go?

[basil-conto]

Thanks for working on this!

Should I expect some sort of confirmation of my CA, or am I good to go?

I think we need to wait until you've heard back from the copyright clerk and have received a copy of your CA (the text).

[Hugo-Heagren]

My CA confirmation came through today, so this can be merged now