Skip to content

Add support for multi-file edits #2359

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fab4100
Copy link

@fab4100 fab4100 commented Dec 2, 2024

Git send-email by default uses multi-file edit mode
(sendemail.multiEdit) when more than one patch file requires editing an
associated email message and/or cover-letters. Multi edit mode spawns
one editor instance with multiple files in argument list. This behavior
is supported by creating multiple splits for a running vim instance when
:G send-email is called. When the user sets sendemail.multiEdit to
false in the local or global config, if multiple files need editing then
they are processed sequentially in a single split at a time.

Closes #2352

@fab4100
Copy link
Author

fab4100 commented Feb 15, 2025

To help push progress, I am describing the changes introduced in this PR in more detail below.

  1. Multiple file arguments are read by adding a for-loop that appends the arguments to the sentinel $FUGITIVE.edit (https://github.com/fab4100/vim-fugitive/blob/28f17e54691def71d5f0bf3aa448fe5bd68b70de/autoload/fugitive.vim#L3958)
  2. Instead of reading a single file argument, a list of arguments is read (https://github.com/fab4100/vim-fugitive/blob/28f17e54691def71d5f0bf3aa448fe5bd68b70de/autoload/fugitive.vim#L3446) which are then processed in a newly added outer loop (https://github.com/fab4100/vim-fugitive/blob/28f17e54691def71d5f0bf3aa448fe5bd68b70de/autoload/fugitive.vim#L3448), each loop iteration initializes a new buffer with focus on the first file argument given upon loop exit (hence the reverse(files) iterator). Since now multiple buffers may be created, it is no longer sufficient to indicate buffer deletion by simply removing the sentinel ($FUGITIVE.edit). Instead, the number of active buffers is stored into the sentinel to keep track of the number of active buffers when multiple files are being edited (https://github.com/fab4100/vim-fugitive/blob/28f17e54691def71d5f0bf3aa448fe5bd68b70de/autoload/fugitive.vim#L3447).
  3. Whenever a buffer is deleted, the active buffer count in the sentinel is decreased https://github.com/fab4100/vim-fugitive/blob/28f17e54691def71d5f0bf3aa448fe5bd68b70de/autoload/fugitive.vim#L3735-L3736. If the active buffer count is zero, the same code is executed as is currently the case for the single file only edit (deleting the sentinel then resume session https://github.com/fab4100/vim-fugitive/blob/28f17e54691def71d5f0bf3aa448fe5bd68b70de/autoload/fugitive.vim#L3739-L3740). If one or more buffers are active, the sentinel is updated and the session is resumed without deleting the sentinel (https://github.com/fab4100/vim-fugitive/blob/28f17e54691def71d5f0bf3aa448fe5bd68b70de/autoload/fugitive.vim#L3742-L3743).
  4. The active buffer count is appended to the resume queue whose arguments are passed down s:RunWait which checks for the newly appended active buffer count argument (see previous item). If there are 1 or more active buffers, s:RunWait returns to caller without taking any action (continuing editing session of active buffers). If the active buffer count is zero the same code is executed as is currently done for the single file only implementation (which concludes the $FUGITIVE.edit session).

Please let me know if I can implement some changes you may desire. Thanks for taking the time.

if bufnr == bufnr('') && !exists('g:fugitive_event')
let files = readfile(sentinel, '')
call writefile([len(files)], sentinel)
for file in reverse(files)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this fair with each of 'splitbelow' and 'nosplitbelow' set? I would expect one to give an unintuitive order.

It's also a little weird that the buffer numbers would end up backwards. The first file would have the biggest buffer number.

(Perfection isn't mandatory here, given that we currently don't support multiple arguments at all.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

splitbelow and nosplitbelow behave as follows with reference to
currently focused window (see <W2> in sketch below):

  • nosplitbelow: stack grows upwards with first file in focus in top most
    split
  • splitbelow: stack grows downwards with first file in focus in bottom
    most split

With reversed sentinel file list iteration the command :G send-email --annotate HEAD~2 results in: (created splits are indicated by S, numbers indicate
relative buffer number)

<  > = focused window                 |  W1  |      |
                                      |------|      |
|  W1  |      |                       | <S2> |      |
|------|      |                       |------|      |
| <W2> |  W4  |  -- nosplitbelow -->  |  S1  |  W4  |
|------|      |                       |------|      |
|  W3  |      |                       |  W2  |      |
                                      |------|      |
                                      |  W3  |      |
<  > = focused window                 |  W1  |      |
                                      |------|      |
|  W1  |      |                       |  W2  |      |
|------|      |                       |------|      |
| <W2> |  W4  |  --  splitbelow  -->  |  S1  |  W4  |
|------|      |                       |------|      |
|  W3  |      |                       | <S2> |      |
                                      |------|      |
                                      |  W3  |      |

The last split is the focused one corresponding to the largest buffer number
holding the first file in the sentinel list. The first file in the sentinel
list corresponds to [PATCH 1/2] which is the focused buffer (with the largest
buffer ID) in this case. Intuitively to me as a user, I would expect that start
editing the first patch and I would be less concerned about the actual buffer ID
relating to this file (while it may be confusing for scripting but I believe :G send-email is mainly interactive). To me this would be the expected behavior.

Removing the file list reversal would result in the same sketch as above (w/r/t
to buffer IDs) but the active buffer <S2> would hold the [PATCH 2/2]. This
would be consistent w/r/t buffer ID <--> file list in sentinel file, but
inconsistent with the file order you are presented when editing the patches
from the terminal with the command git send-email --annotate HEAD~2 (however,
for this case the buffer ID corresponds to the patch ID in the file as well).

I have pushed a commit for in-memory split buffer book-keeping relating to your
comment below (89ed871). In this commit I have removed the reverse file
iteration but switch focus to the first created buffer instead. This is
consistent git send-mail run from the command line and buffer ID <--> patch
ID/sentinel file list. This results in:

<  > = focused window                 |  W1  |      |
                                      |------|      |
|  W1  |      |                       |  S2  |      |
|------|      |                       |------|      |
| <W2> |  W4  |  -- nosplitbelow -->  | <S1> |  W4  |
|------|      |                       |------|      |
|  W3  |      |                       |  W2  |      |
                                      |------|      |
                                      |  W3  |      |
<  > = focused window                 |  W1  |      |
                                      |------|      |
|  W1  |      |                       |  W2  |      |
|------|      |                       |------|      |
| <W2> |  W4  |  --  splitbelow  -->  | <S1> |  W4  |
|------|      |                       |------|      |
|  W3  |      |                       |  S2  |      |
                                      |------|      |
                                      |  W3  |      |

With:

  • S1: file 0001 (a.k.a [PATCH 1/2])
  • S2: file 0002 (a.k.a [PATCH 2/2])

call feedkeys("\<C-\>\<C-N>:redraw!|call delete(" . string(sentinel) .
\ ")|call fugitive#Resume()|checktime\r", 'n')
else
call writefile([active_buffers], sentinel)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not a problem in practice, but it irks me a bit that this file can now contain either a list of files or a number. Can we do this in memory instead? Avoiding I/O is also a win in general.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I certainly agree with I/O argument. I have pushed a change that manages active buffers in memory via the state dictionary (89ed871).

Fabian Wermelinger added 2 commits March 22, 2025 15:36
Git send-email by default uses multi-file edit mode
(`sendemail.multiEdit`) when more than one patch file requires editing
an associated email message and/or cover-letters.  Multi edit mode
spawns one editor instance with multiple files in argument list.  This
behavior is supported by creating multiple splits for a running vim
instance when `:G send-email` is called.  When the user sets
`sendemail.multiEdit` to false in the local or global config, if
multiple files need editing then they are processed sequentially in a
single split at a time.

Closes tpope#2352
Adds an additional 'active_buffers' dictionary to the state dictionary
for tracking of possibly multiple buffers for the edit command.  The
'active_buffers' dictionary has the buffer ID as key and associated file
as value.  The function s:RunEdit initializes the dictionary and
s:RunBufDelete modifies its state.
Added description for multifile edit behavior.
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

Successfully merging this pull request may close these issues.

Edit multiple files / load multiple files into buffer list for :Git <command>'s
2 participants