Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

helm-ff-checksum action should not be available for directories #1769

Closed
xiongtx opened this issue May 12, 2017 · 3 comments
Closed

helm-ff-checksum action should not be available for directories #1769

xiongtx opened this issue May 12, 2017 · 3 comments

Comments

@xiongtx
Copy link
Member

xiongtx commented May 12, 2017

Expected behavior

helm-ff-checksum action, found in the actions list of commands like helm-find-files, should not be available for directories.

Actual behavior from emacs-helm.sh if possible (See note at bottom)

helm-ff-checksum is available for directories. Calling it signals an error:

error in process sentinel: Read error: Is a directory, <dir-name>

but not before prompting the user for the checksum algorithm, which is unnecessary work.

Steps to reproduce (recipe)

  1. Call helm-find-files
  2. Move selection to directory
  3. Call helm-ff-checksum (<f4>)

Backtraces if some (M-x toggle-debug-on-error)

Debugger entered--Lisp error: (file-error "Read error" "Is a directory" "/home/txx/.emacs.d/elpa/helm-20170511.649")
  signal(file-error ("Read error" "Is a directory" "/home/txx/.emacs.d/elpa/helm-20170511.649"))
  async-handle-result(#[257 "\302�!\210\303\304\300\301#\207" [sha256 "helm-20170511.649" kill-new message "Calculating %s checksum for `%s' done and copied to kill-ring"] 5 "\n\n(fn SUM)"] (async-signal (file-error "Read error" "Is a directory" "/home/txx/.emacs.d/elpa/helm-20170511.649")) #<buffer *emacs*>)
  async-when-done(#<process emacs> "finished\n")

Describe versions of helm, emacs, operating system etc.

  • Helm: 20170511.649
  • Emacs: 25.1.1
  • OS: Fedora 24
thierryvolpiatto pushed a commit that referenced this issue May 12, 2017
* helm-files.el (helm-ff-checksum): Do it.
thierryvolpiatto pushed a commit that referenced this issue May 12, 2017
* helm-files.el (helm-find-files-actions): Remove checksum.
(helm-find-files-action-transformer): Add it here.
@thierryvolpiatto
Copy link
Member

Done, thanks.

@xiongtx
Copy link
Member Author

xiongtx commented May 12, 2017

Here's a thought: optional actions should come last, not be injected into the middle.

If we have actions like:

<f1> A
<f2> B
<f3> C

It makes more sense to append an action like:

<f1> A
<f2> B
<f3> C
<f4> D

than inject it like:

<f1> A
<f2> B
<f3> D
<f4> C

because injecting an action shifts the keys for every subsequent action; this requires the user to remember that C is <f4> for files, for example, but <f3> for dirs.

This maybe only applies to actions bound to keys, i.e. <f1>-<f12>.

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented May 13, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants