[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [O] [rfc] org-dired
From: |
Nicolas Goaziou |
Subject: |
Re: [O] [rfc] org-dired |
Date: |
Wed, 15 Nov 2017 12:38:37 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) |
Hello,
Marco Wahl <address@hidden> writes:
> I just pushed the functionality to master.
Thank you.
However, I didn't have time to comment the code.
There are a few stylistic issues:
`mapc' + `lambda' -> `dolist' in `org-attach-attach-files'.
However, I think `org-attach-attach-files' can be removed, since it is
called only once and is really two lines long. IOW, please include it in
`org-attach-dired-attach-to-next-best-subtree'.
"no window in Org-mode" -> "No window displaying an Org buffer"
I don't think it is useful to implement
`org-attach-dired-attach-to-next-best-subtree-mv'. I assume that once
`org-attach-method' is set, a user is unlikely to change it for a single
command. IOW, let's just implement
`org-attach-dired-attach-to-next-best-subtree'.
Nitpick: an inline comment uses a single semicolon.
It would also be better to shorten function names, e.g.
org-attach-dired-attach-to-next-best-subtree -> org-attach-dired-to-subtree
There are also a few issues in "test-org-attach.el".
For example `touch' uses the wrong name-space. Besides, it is not
useful. We usually do
(org-test-with-temp-text-in-file
"... Org bufer..."
(let ((filename (buffer-file-name)))
...))
It would be best to refactor
`test-org-attach/dired-attach-to-next-best-subtree/1' so that `should'
is the outer sexp. The cleanup part should probably be in an
`unwind-protect'. Not that it is not useful if
°org-test-with-temp-text-in-file'.
Regards,
--
Nicolas Goaziou