kuanyui / tldr.el

tldr client for Emacs
142 stars 16 forks source link

fix rename directory error when first time running or update doc #23

Closed showgood closed 6 years ago

showgood commented 6 years ago

when I ran ~tldr~ for the first time, I got error from emacs:

Debugger entered--Lisp error: (file-missing "Renaming" "No such file or directory" "/Users/showgood/.emacs.d/tldr-master" "/Users/showgood/.emacs.d/tldr/tldr-master")
  rename-file("/Users/showgood/.emacs.d/tldr-master" "/Users/showgood/.emacs.d/tldr/")

Reason is because that's the behavior for ~rename-file~ according to elisp doc:

Rename FILE as NEWNAME.  Both args must be strings.
If file has names other than FILE, it continues to have those names.
If NEWNAME is a directory name, rename FILE to a like-named file under
NEWNAME.

basically after this rename-file call, we have the directory:

Users/showgood/.emacs.d/tldr/tldr-master

instead of

Users/showgood/.emacs.d/tldr/

didn't find an easy way to fix in elisp, but using mv would fix it.

kuanyui commented 6 years ago

Hmm...I guess this won't work under Windows... Maybe need to find another way.

showgood commented 6 years ago

as long as there is mv command on the OS, it should be fine. This package already requires unzip command, so it's not too ridiculous to require mv available.

also most emacs user already have something like cygwin installed, this is not really an issue. Many emacs packages require that kind of dependency.

I would also imagine whoever interested in this package would be mainly mac/linux user, if they do use windows, they should have no problem with CLI tools given the purpose of this package.

Given all this being said, I understood if you still have concern and don't want to take this PR.

kuanyui commented 6 years ago

That sounds reasonable. But you still need to deal with the path with spaces. e.g.:(shell-command (format "mv '%s' '%s'" ...))

showgood commented 6 years ago

good point. I have updated the PR.