From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marco Wahl <marcowahlsoft@gmail.com>
Subject: Re: [rfc] org-dired
Date: Wed, 15 Nov 2017 14:54:28 +0100
Message-ID: <841skzhckr.fsf@gmail.com>
References: <84bmk7ttkh.fsf@gmail.com> <87mv3r1mgh.fsf@nicolasgoaziou.fr>
	<84inebztpf.fsf@gmail.com> <8760abzs8y.fsf@nicolasgoaziou.fr>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Return-path: <emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:54951)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <marcowahlsoft@gmail.com>) id 1eEy9G-0002wb-0B
	for emacs-orgmode@gnu.org; Wed, 15 Nov 2017 08:54:38 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <marcowahlsoft@gmail.com>) id 1eEy9A-0002jt-6N
	for emacs-orgmode@gnu.org; Wed, 15 Nov 2017 08:54:38 -0500
Received: from mail-wm0-x230.google.com ([2a00:1450:400c:c09::230]:37083)
	by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)
	(Exim 4.71) (envelope-from <marcowahlsoft@gmail.com>)
	id 1eEy99-0002iT-VC
	for emacs-orgmode@gnu.org; Wed, 15 Nov 2017 08:54:32 -0500
Received: by mail-wm0-x230.google.com with SMTP id v186so3135297wma.2
	for <emacs-orgmode@gnu.org>; Wed, 15 Nov 2017 05:54:31 -0800 (PST)
List-Id: "General discussions about Org-mode." <emacs-orgmode.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/emacs-orgmode>,
	<mailto:emacs-orgmode-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/emacs-orgmode/>
List-Post: <mailto:emacs-orgmode@gnu.org>
List-Help: <mailto:emacs-orgmode-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/emacs-orgmode>,
	<mailto:emacs-orgmode-request@gnu.org?subject=subscribe>
Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org
Sender: "Emacs-orgmode"
	<emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org>
To: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Cc: emacs-orgmode@gnu.org

Hi!

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Marco Wahl <marcowahlsoft@gmail.com> 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'.

Done.

> 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'.

Done.

>   "no window in Org-mode" -> "No window displaying an Org buffer"

Done.

> 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'.

Done.

> Nitpick: an inline comment uses a single semicolon.

Okay.

> It would also be better to shorten function names, e.g.
>
>   org-attach-dired-attach-to-next-best-subtree  ->  org-attach-dired-to-s=
ubtree

Done.

> There are also a few issues in "test-org-attach.el".
>
> For example `touch' uses the wrong name-space. Besides, it is not
> useful.

Removed.

> We usually do
>
>   (org-test-with-temp-text-in-file
>     "... Org bufer..."
>     (let ((filename (buffer-file-name)))
>      ...))

Okay.

> 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
> =C2=B0org-test-with-temp-text-in-file'.

I followed you suggestions and I think the new version is cleaner.


Thank you!
          Marco