Hi,

I wanted to let the user get a list of tmr commands when calling
embark-act from a timer line in the tmr-tabulated view.

For this, I wrote this code:

    (defun my/tmr-tabulated-target-timer-at-point ()
      "Target the timer on the current line in a tmr-tabulated buffer."
      (when (derived-mode-p 'tmr-tabulated-mode)
        (when-let ((timer (tabulated-list-get-id)))
          `(
            tmr-timer
            ,(substring-no-properties (tmr--long-description timer))
            ,(line-beginning-position) . ,(line-end-position)))))

    (add-to-list 'embark-target-finders #'my/tmr-tabulated-target-timer-at-point)

I can now type C-S-a (my key for embark-act) on a timer line and get a
list of embark actions I can call on the timer at point. This seems to
work for all commands except for #'tmr-edit-description which never asks
for a description but generate a weird one automatically.

What am I missing?

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

> I wanted to let the user get a list of tmr commands when calling
> embark-act from a timer line in the tmr-tabulated view.
> ...
> I can now type C-S-a (my key for embark-act) on a timer line and get a
> list of embark actions I can call on the timer at point. This seems to
> work for all commands except for #'tmr-edit-description which never asks
> for a description but generate a weird one automatically.
> 
> What am I missing?

I assume that you observe an unfortunate interaction of
tmr--read-timer-hook and the minibuffer injection. When acting in the
tabulated mode buffer, the hook is used to read the timer at point, such
that Embark doesn't even get a chance to inject the timer. Instead the
timer is injection as description, which leads to the weird description.

We could fix this by removing tmr--read-timer-hook again and
reintroducing all the wrapper commands, e.g., tmr-tabulated-cancel.

(defun tmr-tabulated-cancel ()
  (interactive)
  (tmr-cancel (tmr-tabulated--at-point)))

However there is no point in using Embark in the tabulated buffer, since
all the commands are already available as direct keybindings. Therefore
I'd say we just leave the design as is.

Daniel

Hi Daniel,

Daniel Mendler <mail@daniel-mendler.de> writes:
> I assume that you observe an unfortunate interaction of
> tmr--read-timer-hook and the minibuffer injection. When acting in the
> tabulated mode buffer, the hook is used to read the timer at point, such
> that Embark doesn't even get a chance to inject the timer. Instead the
> timer is injection as description, which leads to the weird description.


this makes perfect sense, you are right.


> However there is no point in using Embark in the tabulated buffer, since
> all the commands are already available as direct keybindings. Therefore
> I'd say we just leave the design as is.


I agree it would be a shame to reintroduce all this mostly stupid
commands, your new implementation is so much better. Still, I don't
agree when you say "there is no point in using Embark in the tabulated
buffer". I see 2 advantages:

- consistency: I can use embark from the minibuffer and from the
  tabulated view and get exactly the same interface;

- discoverability: embark displays a small list of actions (with
  bindings) that make sense. It's easier to remember the embark-act
  shortcut than to remember all shortcuts of several Emacs apps.

That being said, I see no way forward to solve this issue.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

>> However there is no point in using Embark in the tabulated buffer, since
>> all the commands are already available as direct keybindings. Therefore
>> I'd say we just leave the design as is.
> 
> I agree it would be a shame to reintroduce all this mostly stupid
> commands, your new implementation is so much better. Still, I don't
> agree when you say "there is no point in using Embark in the tabulated
> buffer". I see 2 advantages:
> 
> - consistency: I can use embark from the minibuffer and from the
>   tabulated view and get exactly the same interface;

I agree that consistency is important, but consistency is already
realized to some extent without special support. If you compare an
embark-collect buffer, you can enable the
embark-collect-direct-action-minor-mode. Then you can access the actions
directly without calling embark-act first. The collect buffer will then
behave in almost the same way as the tabulated view already does.

Embark does not aim to replace Dired, Ibuffer, the tabulated view etc.
The aim of Embark is to provide minibuffer actions and to provide
actions at point in text buffers. But if you already have a specialized
buffer like the TMR tabulated view, this buffer is usually preferable as
in our case here.

> - discoverability: embark displays a small list of actions (with
>   bindings) that make sense. It's easier to remember the embark-act
>   shortcut than to remember all shortcuts of several Emacs apps.

I agree that discoverability is an important advantage of Embark, but
again discoverability is already realized without Embark. You can press
h to get the mode help which will display all mode-specific commands.
Alternatively you can invoke embark-bindings to get a list of all
bindings in the buffer using completing-read.

I recommend to bind embark-bindings to a convenient key, e.g., C-h B.
You can also search through local bindings only using C-u C-h B. If this
discoverability aspect isn't good enough then we should generally ask
the question why mode-specific bindings are perceived as
non-discoverable or less discoverable than Embark actions. I don't think
they are, but maybe you can elaborate?

There is another argument to be made - Embark unifies actions across
different modes. This means that Embark even has some value in Dired or
Ibuffer modes. There we can still obtain the file/buffer candidates
(embark-collect, embark-export) and also operate on them using all
Embark file/buffer actions. However in our case this advantage does not
play a role. TMRs are not files, they only exist in the TMR minibuffer
and the TMR tabulated view, nowhere else. This also means that we
probably don't want to add custom actions for TMRs. In contrast, file
paths exist practically everywhere in Emacs at different places such
that one profits from the ubiquitous availability of actions.

> That being said, I see no way forward to solve this issue.

As mentioned, one solution would be to reintroduce the tmr-tabulated-*
wrapper commands. Another alternative would be to somehow disable the
tmr--read-timer-hook temporarily via embark-pre/post-action hooks (or
via a hypothetical around hook which you proposed on the Embark issue
tracker). Using the Embark hooks wouldn't require changes on the side of
TMR.

Overall the Embark support we have now is sufficient in my opinion.
We've achieved Embark support in the completing-read minibuffer which
brings this buffer to the level of the tabulated view. Furthermore the
embark-collect buffer obtained from completing-read is comparable to the
tabulated view. The only missing features are the live update of the
Remaining column and the generally nicer column view of the tabulated view.

Enabling Embark at point support in tabulated buffers is mostly an
exercise for completeness, but doesn't offer real practical benefits. If
you prefer the Embark workflow everywhere you could also scrap the TMR
tabulated view, IBuffer, Dired and all the specialized buffers. Just
always use completing-read as entry point and use the Embark collect
buffer if the candidates should live a bit longer.

Daniel

On 6/30/22 18:55, Daniel Mendler wrote:
> As mentioned, one solution would be to reintroduce the tmr-tabulated-*
> wrapper commands. Another alternative would be to somehow disable the
> tmr--read-timer-hook temporarily via embark-pre/post-action hooks (or
> via a hypothetical around hook which you proposed on the Embark issue
> tracker). Using the Embark hooks wouldn't require changes on the side of
> TMR.

As a proof of concept - the following works and fixes
tmr-edit-description if triggered via Embark:

(add-to-list 'embark-pre-action-hooks '(tmr-edit-description
tmr-tabulated-pre-embark))
(add-to-list 'embark-post-action-hooks '(tmr-edit-description
tmr-tabulated-post-embark))
(add-to-list 'embark-target-finders
#'my/tmr-tabulated-target-timer-at-point)

(defun tmr-tabulated-pre-embark (&rest _)
  (when (eq major-mode 'tmr-tabulated-mode)
    (setq tmr--read-timer-hook nil)))

(defun tmr-tabulated-post-embark (&rest _)
  (when (eq major-mode 'tmr-tabulated-mode)
    (setq tmr--read-timer-hook '(tmr-tabulated--timer-at-point))))

(defun my/tmr-tabulated-target-timer-at-point ()
  "Target the timer on the current line in a tmr-tabulated buffer."
  (when (derived-mode-p 'tmr-tabulated-mode)
    (when-let ((timer (tabulated-list-get-id)))
      `(
        tmr-timer
        ,(substring-no-properties (tmr--long-description timer))
        ,(line-beginning-position) . ,(line-end-position)))))

Daniel