~protesilaos/denote

9 2

denote-dired-rename-marked-files-using-front-matter and dired-vc-rename-file

Details
Message ID
<166547153518.8.941129310186454444.68125516@aboulafia.org>
DKIM signature
missing
Download raw message
Hi!

My denote silo is version controlled (via Git). Moreover, `dired-vc-
rename-file' is set to t in my configuration: when I manually rename a
file using Dired, "git mv" is used (instead of just "mv").

However, when I used `denote-dired-rename-marked-files-using-front-
matter', I noticed that renaming was done without using "git mv" (Git
showed that a file was a deleted, and another one was created, without
staging). I think it's a small bug, and that `denote-dired-rename-
marked-files*' functions should use "git mv" (or any other command for
other VC system) if `dired-vc-rename-file' is set.

What do you think about that?

Regards,
-- 
Florian
Details
Message ID
<87edve4yr3.fsf@protesilaos.com>
In-Reply-To
<166547153518.8.941129310186454444.68125516@aboulafia.org> (view parent)
DKIM signature
missing
Download raw message
Patch: +5 -1
> From: florian.denote@aboulafia.org
> Date: Tue, 11 Oct 2022 06:58:50 +0000
>
> Hi!

Good day Florian!

> My denote silo is version controlled (via Git). Moreover, `dired-vc-
> rename-file' is set to t in my configuration: when I manually rename a
> file using Dired, "git mv" is used (instead of just "mv").
>
> However, when I used `denote-dired-rename-marked-files-using-front-
> matter', I noticed that renaming was done without using "git mv" (Git
> showed that a file was a deleted, and another one was created, without
> staging). I think it's a small bug, and that `denote-dired-rename-
> marked-files*' functions should use "git mv" (or any other command for
> other VC system) if `dired-vc-rename-file' is set.
>
> What do you think about that?

I agree.  Currently we are using the standard 'rename-file'.  What I am
not sure about is whether we should make the behaviour conditional or
not.

Here is a SAMPLE diff:


 denote.el | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/denote.el b/denote.el
index 1cbba9e..16147cf 100644
--- a/denote.el
+++ b/denote.el
@@ -1628,10 +1628,14 @@ (defun denote--rename-buffer (old-name new-name)
    (with-current-buffer buffer
      (set-visited-file-name new-name nil t))))

(defvar dired-vc-rename-file)

(defun denote-rename-file-and-buffer (old-name new-name)
  "Rename file named OLD-NAME to NEW-NAME, updating buffer name."
  (unless (string= (expand-file-name old-name) (expand-file-name new-name))
    (rename-file old-name new-name nil)
    (if (and (derived-mode-p 'dired-mode) dired-vc-rename-file)
        (vc-rename-file old-name new-name)
      (rename-file old-name new-name nil))
    (denote--rename-buffer old-name new-name)))

(define-obsolete-function-alias


Any thoughts?

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<166602452217.8.15435838709553607742.69540320@aboulafia.org>
In-Reply-To
<87edve4yr3.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
Protesilaos Stavrou wrote:
> -    (rename-file old-name new-name nil)
> +    (if (and (derived-mode-p 'dired-mode) dired-vc-rename-file)
> +        (vc-rename-file old-name new-name)
> +      (rename-file old-name new-name nil))
>      (denote--rename-buffer old-name new-name)))
>  
>  (define-obsolete-function-alias
> 
> 
> Any thoughts?

Hi Prot,

Thank you for your proposition. I don't think it will work when files
are not under version control and dired-vc-rename-file is set to t.

I tried and Emacs throwed an exception:

   vc-rename-file: Please update files before moving them

Maybe we could use dired-rename-file for renaming files when current
mode is dired-mode, and let this function take care of dired-vc-rename-
file value?

Something like that:

(if (derived-mode-p 'dired-mode)
    (dired-rename-file old-name new-name)
    (rename-file old-name new-name nil))

What's your opinion about that?

Cheers,
-- 
Florian
Details
Message ID
<87r0z5u7k5.fsf@protesilaos.com>
In-Reply-To
<166602452217.8.15435838709553607742.69540320@aboulafia.org> (view parent)
DKIM signature
missing
Download raw message
> From: florian.denote@aboulafia.org
> Date: Mon, 17 Oct 2022 16:35:17 +0000
>
> Protesilaos Stavrou wrote:
>> -    (rename-file old-name new-name nil)
>> +    (if (and (derived-mode-p 'dired-mode) dired-vc-rename-file)
>> +        (vc-rename-file old-name new-name)
>> +      (rename-file old-name new-name nil))
>>      (denote--rename-buffer old-name new-name)))
>>  
>>  (define-obsolete-function-alias
>> 
>> 
>> Any thoughts?
>
> Hi Prot,

Hello Florian,

> Thank you for your proposition. I don't think it will work when files
> are not under version control and dired-vc-rename-file is set to t.
>
> I tried and Emacs throwed an exception:
>
>    vc-rename-file: Please update files before moving them
>
> Maybe we could use dired-rename-file for renaming files when current
> mode is dired-mode, and let this function take care of dired-vc-rename-
> file value?
>
> Something like that:
>
> (if (derived-mode-p 'dired-mode)
>     (dired-rename-file old-name new-name)
>     (rename-file old-name new-name nil))
>
> What's your opinion about that?

Yes, this makes sense.  Note that the 'dired-rename-file' is missing a
final OK-IF-ALREADY-EXISTS argument.

Do you want to prepare a patch or shall I do it from here?

All the best,
Prot

-- 
Protesilaos Stavrou
https://protesilaos.com

[PATCH] Re: denote-dired-rename-marked-files-using-front-matter and dired-vc-rename-file

Details
Message ID
<166609614637.7.16696322729257093331.69714697@aboulafia.org>
In-Reply-To
<87r0z5u7k5.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
Le mardi 18 octobre 2022 à 06:36 +0300, Protesilaos Stavrou -
info(a)protesilaos.com a écrit :
> Do you want to prepare a patch or shall I do it from here?

Hi Prot,

I just prepared the patch, it's attached to this mail. Hope it's OK!

Cheers,
-- 
Florian

Re: [PATCH] Re: denote-dired-rename-marked-files-using-front-matter and dired-vc-rename-file

Details
Message ID
<87ilkhmh5k.fsf@protesilaos.com>
In-Reply-To
<166609614637.7.16696322729257093331.69714697@aboulafia.org> (view parent)
DKIM signature
missing
Download raw message
> From: florian.denote@aboulafia.org
> Date: Tue, 18 Oct 2022 12:29:03 +0000

> [... 6 lines elided]

> I just prepared the patch, it's attached to this mail. Hope it's OK!

> [... 31 lines elided]

Thank you!  I installed the patch and updated the manual's
Acknowledgements to mention your name.

-- 
Protesilaos Stavrou
https://protesilaos.com

Re: [PATCH] Re: denote-dired-rename-marked-files-using-front-matter and dired-vc-rename-file

Details
Message ID
<87fsflmgn9.fsf@protesilaos.com>
In-Reply-To
<87ilkhmh5k.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
> From: Protesilaos Stavrou <info@protesilaos.com>
> Date: Tue, 18 Oct 2022 15:48:39 +0300
>
>> From: florian.denote@aboulafia.org
>> Date: Tue, 18 Oct 2022 12:29:03 +0000
>
>> [... 6 lines elided]
>
>> I just prepared the patch, it's attached to this mail. Hope it's OK!
>
>> [... 31 lines elided]
>
> Thank you!  I installed the patch and updated the manual's
> Acknowledgements to mention your name.

Just to note that I made a follow-up commit which changes the code thus:

    (defun denote-rename-file-and-buffer (old-name new-name)
      "Rename file named OLD-NAME to NEW-NAME, updating buffer name."
      (unless (string= (expand-file-name old-name) (expand-file-name new-name))
        (cond
         ((derived-mode-p 'dired-mode)
          (dired-rename-file old-name new-name nil))
         ((vc-backend old-name)
          (vc-rename-file old-name new-name))
         (t
          (rename-file old-name new-name nil)))
        (denote--rename-buffer old-name new-name)))


    commit 21e194a5bdedd26a451dab4c49ef7a177ff78482
    Author: Protesilaos Stavrou <info@protesilaos.com>
    Date:   Tue Oct 18 15:52:48 2022 +0300

        Refine file renaming under version control

        This expands the conditionality introduced by Florian in commit
        234520b.  Read the mailing list for the relevant discussion:
        <https://lists.sr.ht/~protesilaos/denote/%3C166547153518.8.941129310186454444.68125516@aboulafia.org%3E>.

     denote.el | 10 +++++++---
     1 file changed, 7 insertions(+), 3 deletions(-)


I think this covers all use-cases now, right?

-- 
Protesilaos Stavrou
https://protesilaos.com

Re: [PATCH] Re: denote-dired-rename-marked-files-using-front-matter and dired-vc-rename-file

Details
Message ID
<166610597107.7.2816311561595757629.69754358@aboulafia.org>
In-Reply-To
<87fsflmgn9.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
>          ((vc-backend old-name)
>           (vc-rename-file old-name new-name))

Thank you for applying the patch.

I'm not sure about this one: vc-rename-file doesn't respect dired-vc-
rename-file (of course), so if a file is under version control, and
dired-vc-rename-file is nil:

- if denote-rename-file-and-buffer is called from a Dired buffer, the
file is renamed without using "git mv"
- if denote-rename-file-and-buffer is called from a non-Dired buffer
(for example the buffer visiting the file), the file is renamed using
"git mv"

I'm OK with that (because I want all my files to be renamed using "git
mv"), but I think some users could find in inconsistent.

Cheers,
-- 
Florian

Re: [PATCH] Re: denote-dired-rename-marked-files-using-front-matter and dired-vc-rename-file

Details
Message ID
<87a65tp2v1.fsf@protesilaos.com>
In-Reply-To
<166610597107.7.2816311561595757629.69754358@aboulafia.org> (view parent)
DKIM signature
missing
Download raw message
> From: florian.denote@aboulafia.org
> Date: Tue, 18 Oct 2022 15:12:35 +0000
>
>>          ((vc-backend old-name)
>>           (vc-rename-file old-name new-name))
>
> Thank you for applying the patch.

You are welcome!

> I'm not sure about this one: vc-rename-file doesn't respect dired-vc-
> rename-file (of course), so if a file is under version control, and
> dired-vc-rename-file is nil:
>
> - if denote-rename-file-and-buffer is called from a Dired buffer, the
> file is renamed without using "git mv"
> - if denote-rename-file-and-buffer is called from a non-Dired buffer
> (for example the buffer visiting the file), the file is renamed using
> "git mv"
>
> I'm OK with that (because I want all my files to be renamed using "git
> mv"), but I think some users could find in inconsistent.

Correct.  I don't think there is a practical problem here.  A 'git mv'
instead of 'mv' feels innocuous.  Do you think there is a workflow that
is harmed by 'git mv' but is fine with 'mv'?

This inconsistency is not our fault, anyway: it existed before as well
when we introduced the conditionality for Dired.  It would be better if
Emacs had a uniform interface for all the "rename" commands to
optionally respect the underlying version control system, where
applicable.

-- 
Protesilaos Stavrou
https://protesilaos.com

Re: [PATCH] Re: denote-dired-rename-marked-files-using-front-matter and dired-vc-rename-file

Details
Message ID
<166611133580.8.18263751751744420854.69777145@aboulafia.org>
In-Reply-To
<87a65tp2v1.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
Le mardi 18 octobre 2022 à 18:29 +0300, Protesilaos Stavrou -
info(a)protesilaos.com a écrit :
> Correct.  I don't think there is a practical problem here.  A 'git
> mv' instead of 'mv' feels innocuous.  Do you think there is a
> workflow that is harmed by 'git mv' but is fine with 'mv'?

I don't think so.

> This inconsistency is not our fault, anyway: it existed before as
> well when we introduced the conditionality for Dired.  It would be
> better if Emacs had a uniform interface for all the "rename" commands
> to optionally respect the underlying version control system, where
> applicable.

I agree with you!

Regards,
-- 
Florian
Reply to thread Export thread (mbox)