Hi,
Very nice package and I expect it to be very useful.
I'm Just an end-user so I don't understand Christian's explanation, but
that is the first thing I noticed - that it doesn't work with the up and
down arrow keys.
Also, if you land in a directory by doing p, p, etc, then <enter> on the
folder name, the point lands on the first file listed. Preview does not
work there - maybe that is intended.
Ed Hamilton
edhamilton9@gmail.com
> From: Ed Hamilton <edhamilton9@gmail.com>> Date: Sun, 25 Jun 2023 16:12:50 -0400>> Hi,
Hello Ed, Christian,
> Very nice package and I expect it to be very useful.
You are welcome!
> I'm Just an end-user so I don't understand Christian's explanation, but > that is the first thing I noticed - that it doesn't work with the up and > down arrow keys.
What Christian suggests is that instead of triggering the effect via a
command bound to a key, we do it automatically after the 'dired-mark' is
set.
Christian, since you already have the proposal ready, do you want to
prepare a patch for it?
> Also, if you land in a directory by doing p, p, etc, then <enter> on the > folder name, the point lands on the first file listed. Preview does not > work there - maybe that is intended.
Good to know! Maybe Christian's approach will work in this case.
All the best,
Protesilaos (or simply "Prot")
--
Protesilaos Stavrou
https://protesilaos.com
Hi Prot & Ed!
>> I'm Just an end-user so I don't understand Christian's explanation, but >> that is the first thing I noticed - that it doesn't work with the up and >> down arrow keys.>> What Christian suggests is that instead of triggering the effect via a> command bound to a key, we do it automatically after the 'dired-mark' is> set.
This would be the ideal, but dired.el doesn't notify "interested parties"
when the selected line changes. There's no hook, for some reason. This
could be a useful addition upstream in the Emacs source code, but at the
moment, we're stuck with key rebindings.
> Christian, since you already have the proposal ready, do you want to> prepare a patch for it?
With these limitations in mind, please find a patch attached that shows
how I used to "fix" updating the preview for marking filed with keys
like `d' or `m'.
Code review questions:
- Should we use a lambda instead of a named function?
I'll think about key rebindings a bit more in the meantime :)
Cheers,
Christian
--
Sent from Bielefeld, Germany <3
https://christiantietze.de -- Programming + Personal
https://zettelkasten.de -- Creative Knowledge Work
> This would be the ideal, but dired.el doesn't notify "interested parties"> when the selected line changes. There's no hook, for some reason. This> could be a useful addition upstream in the Emacs source code, but at the> moment, we're stuck with key rebindings.
One more thing:
`dired-move-to-filename' is apparently called after most (all?)
selection and movement commands, so maybe we could figure out to advise
this function instead. Doing it right now will result in a stack
overflow of sorts because the preview calls `dired-move-to-filename',
too.
Another suggestion would be to grep for all callers of this function in
dired.el, assemble a list of these functions, and advise each and every
one of them.
That's quite brittle, though. And similar to key rebinding, it will
not recognize user functions that modify the selected line.
Maybe key rebinding *is* the best way at the moment?
-- Christian
--
Sent from Bielefeld, Germany <3
https://christiantietze.de -- Programming + Personal
https://zettelkasten.de -- Creative Knowledge Work
> From: Christian Tietze <me@christiantietze.de>> Date: Mon, 26 Jun 2023 12:24:42 +0200>>> This would be the ideal, but dired.el doesn't notify "interested parties">> when the selected line changes. There's no hook, for some reason. This>> could be a useful addition upstream in the Emacs source code, but at the>> moment, we're stuck with key rebindings.>> One more thing:>> [... 13 lines elided]>> Maybe key rebinding *is* the best way at the moment?
Thank you, Christian! I installed your patch. I think the advice
mechanism is better suited for this purpose. With the ":after", we are
using it in practically the same way as a hook. I thus generalised what
you did in the following commit:
commit 3304286c175387602032a15a7d1888473ca054b0
Author: Protesilaos Stavrou <info@protesilaos.com>
Date: Tue Jun 27 06:42:04 2023 +0300
Use an advice instead of defining new commands; delete our keymap
This is the same ideas as in commit ae93720, except more generalised.
Instead of defining our own commands, we advise Dired to display a
preview after one of its standard commands is called. This way we
avoid the problem where the key remap mechanism does not actually
remap all keys associated with a given command.[1][2]
[1] Thanks to Peter Prevos for reporting this in issue 1 on the GitHub
mirror: <https://github.com/protesilaos/dired-preview/issues/1>.
[2] Thanks to Christian Tietze and Ed Hamilton for discussing this
with me on the mailing list:
<https://lists.sr.ht/~protesilaos/general-issues/%3Cm1zg4noej2.fsf%40christiantietze.de%3E>.
Commit ae93720 by Christian Tietze is based on this discussion.
dired-preview.el | 50 +++++++++++++-------------------------------------
1 file changed, 13 insertions(+), 37 deletions(-)
Please give it a try.
We may still need to tweak how this behaves, based on what Ed reported
in this thread.
--
Protesilaos Stavrou
https://protesilaos.com
With the patch, dired-preview works with both arrow keys. Also with prefixes like C-u 11 n, or C-11 p, or c-11 up or down arrow key, etc.
It still does not find the first file in a directory if you land in that directory in the way I described earlier. It leaves the most recently displayed file still displayed. If you move down a file or more and then back up, it will display the first file.
Also, I just noticed that if
- you have dired-preview-mode active but are not in dired, and
- you are editing a file and
- then do C-x d to open dired in the file's directory, and
- you land on a file, then dired-preview does not display the file.
Please note that I am mentioning these things because I feel you are interested in getting the package to work well. For myself, I could easily work around the small problem of not finding the first file in a directory.
-- Ed H
Protesilaos Stavrou <info@protesilaos.com> writes:
>> From: Christian Tietze <me@christiantietze.de>>> Date: Mon, 26 Jun 2023 12:24:42 +0200>>>>> This would be the ideal, but dired.el doesn't notify "interested parties">>> when the selected line changes. There's no hook, for some reason. This>>> could be a useful addition upstream in the Emacs source code, but at the>>> moment, we're stuck with key rebindings.>>>> One more thing:>>>> [... 13 lines elided]>>>> Maybe key rebinding *is* the best way at the moment?>> Thank you, Christian! I installed your patch. I think the advice> mechanism is better suited for this purpose. With the ":after", we are> using it in practically the same way as a hook. I thus generalised what> you did in the following commit:>> commit 3304286c175387602032a15a7d1888473ca054b0> Author: Protesilaos Stavrou <info@protesilaos.com>> Date: Tue Jun 27 06:42:04 2023 +0300>> Use an advice instead of defining new commands; delete our keymap>> This is the same ideas as in commit ae93720, except more generalised.> Instead of defining our own commands, we advise Dired to display a> preview after one of its standard commands is called. This way we> avoid the problem where the key remap mechanism does not actually> remap all keys associated with a given command.[1][2]>> [1] Thanks to Peter Prevos for reporting this in issue 1 on the GitHub> mirror: <https://github.com/protesilaos/dired-preview/issues/1>.>> [2] Thanks to Christian Tietze and Ed Hamilton for discussing this> with me on the mailing list:> <https://lists.sr.ht/~protesilaos/general-issues/%3Cm1zg4noej2.fsf%40christiantietze.de%3E>.> Commit ae93720 by Christian Tietze is based on this discussion.>> dired-preview.el | 50 +++++++++++++-------------------------------------> 1 file changed, 13 insertions(+), 37 deletions(-)>> Please give it a try.>> We may still need to tweak how this behaves, based on what Ed reported> in this thread.
--
Ed Hamilton
> From: Ed Hamilton <edhamilton9@gmail.com>> Date: Tue, 27 Jun 2023 05:06:08 -0400>> With the patch, dired-preview works with both arrow keys. Also with> prefixes like C-u 11 n, or C-11 p, or c-11 up or down arrow key, etc.
Good to know. Thank you!
> It still does not find the first file in a directory if you land in> that directory in the way I described earlier. It leaves the most> recently displayed file still displayed. If you move down a file or> more and then back up, it will display the first file.
Indeed. I have noticed those issues. I am not sure how to handle the
"landing" phase because there is no hook that runs when a new window is
focused. It must be doable, but will need more work.
> Also, I just noticed that if> - you have dired-preview-mode active but are not in dired, and > - you are editing a file and> - then do C-x d to open dired in the file's directory, and> - you land on a file, then dired-preview does not display the file.
I tried this right now but could not reproduce it. Maybe I fixed it
by accident in one of the subsequent commits I made?
> Please note that I am mentioning these things because I feel you are> interested in getting the package to work well. For myself, I could> easily work around the small problem of not finding the first file in> a directory.
You are doing well and I appreciate your feedback. Yes, I want to make
this work as reliably and intuitively as possible. Thank you!
--
Protesilaos Stavrou
https://protesilaos.com