Skip to content

refactor(send): replace attachment scratch buffer with buffer-local commands#20

Open
yousefakbar wants to merge 1 commit intomainfrom
feat/compose-attachments
Open

refactor(send): replace attachment scratch buffer with buffer-local commands#20
yousefakbar wants to merge 1 commit intomainfrom
feat/compose-attachments

Conversation

@yousefakbar
Copy link
Copy Markdown
Owner

Replaces the separate scratch buffer approach for managing compose
attachments with a buffer-local variable and dedicated commands,
matching the pattern used for reply attachments.

Changes:

  • Remove buf_attach scratch buffer and attachment_window keymap
  • Add buffer-local 'notmuch_attachments' variable to track attachments
  • Introduce :Attach, :AttachRemove, and :AttachList commands
  • Use attach_cmd module handlers for attachment management
  • Remove old build_mime_msg() in favor of build_mime_msg_from_attachments()

This provides a more consistent and cleaner interface for handling
attachments across both compose and reply workflows.

…ommands

Replaces the separate scratch buffer approach for managing compose
attachments with a buffer-local variable and dedicated commands,
matching the pattern used for reply attachments.

Changes:
- Remove buf_attach scratch buffer and attachment_window keymap
- Add buffer-local 'notmuch_attachments' variable to track attachments
- Introduce :Attach, :AttachRemove, and :AttachList commands
- Use attach_cmd module handlers for attachment management
- Remove old build_mime_msg() in favor of build_mime_msg_from_attachments()

This provides a more consistent and cleaner interface for handling
attachments across both compose and reply workflows.
@yousefakbar
Copy link
Copy Markdown
Owner Author

Hi @simonhughxyz !

Following my merge comment on #7 , I have made the same buffer-local variable + command driven flow for attachment handling that I did for reply messages the go-to method for compose messages as well, overriding your scratch buffer approach.

I'd love your thoughts on this approach since you requested the ability to add attachments.

Let me know if this approach works better than the scratch buffer -- I tend to lean towards this one as it feels more user-friendly and intuitive to vim-isms, but curious to know your thoughts first before/if I merge.

@jugarpeupv
Copy link
Copy Markdown
Contributor

jugarpeupv commented Dec 31, 2025

@yousefakbar I tested your branch, it is awesome to be able to send emails with attachments from inside neovim thanks to your plugin!

However i found that using Attach + hitting tab to select the file from only the current directory is not very ergonomic

Ideally, we could define in the config, in which directories the search for attachments should be performed, and then, once you invoke Attach, then spawn the picker (telescope or snacks) to select files from that directory

@simonhughxyz
Copy link
Copy Markdown
Contributor

@yousefakbar I have not had a chance to review your modifications yet as its the holidays but I have seen it and will get to it when I can. Happy new year!

@yousefakbar
Copy link
Copy Markdown
Owner Author

Hey @jugarpeupv,

However i found that using Attach + hitting tab to select the file from only the current directory is not very ergonomic

I'd love to know more details about this.

Right now, if you type :Attach followed by <Tab>, it will show files in the current directory (default behavior with the 'file' completion builtin).

But if you want attachments from another directory you could always tab-complete that too with for example:

:Attach ../<Tab> " autocompletes with files from parent directory

Or:

:Attach ~/Downlo<Tab>/<Tab> " autocompletes with files from your home Downloads directory

Did you have another flow in mind? I'm curious why you'd set up a default directory for files to pick as attachments? Do you only attach files from one directory?

@jugarpeupv
Copy link
Copy Markdown
Contributor

jugarpeupv commented Jan 9, 2026

@yousefakbar well, you are right about that it is better not to narrow down to a list of directories to search for attachments

Ideally we could support both options (we wil discuss implemenation after):

Option 1: You specify the full path to the attachment file --> Will use current approach (:Attach ~/Download/<TAB>)
Option 2 (the default): You invoke a picker (like telescope or snack) and feed the files present in a list of common directories configured in the plugins config. This approach for me is better, because you can fuzzy search directly for the files that you want, without knowing or typing the full path where they are placed

Or even a combination of both in which, in case you run :Attach ~/Documents (you dont specify a file), then a picker will be presented with all the files available in that directory so you can pick which file you want fuzzy finding

@jugarpeupv
Copy link
Copy Markdown
Contributor

@yousefakbar anyway, i would like to get this feature merged like it is now, so we can attach files. The fuzzy picker approach could be implemented afterwards

is there something needed for this PR to get merged?

@yousefakbar
Copy link
Copy Markdown
Owner Author

@jugarpeupv In its current state it's good to go to be merged. I wasn't rushing because I was curious for feedback from you and @simonhughxyz who initially implemented #7 , which would be deprecated by this.

The fuzzy picker approach could be implemented afterwards

In general I'm thinking of many use cases for fuzzy pickers (specifically telescope.nvim) like filtering, tagging, drafts management, etc. and now for document management as well as you mentioned.

I'll open up an issue so that we can brainstorm and collect all ideas.

@simonhughxyz
Copy link
Copy Markdown
Contributor

Sorry for the late reply, been really busy.
The advantage of using a separate scratch buffer to add attachments was that you can use your own workflow to add files, as its a simple vim buffer you can use autocomplete, telescope, tree picker, snippets or any other method to get a file path and insert it into the buffer, it follows the simple unix and vim convention that its always better to deal with text (I tend to prefer simple solutions to problems).

Here is a user case, you use snippets to create email templates but also attachment templates, suppose you send lots of emails and you often include common PDF's, such as company leaflets etc Having the attachment window be a common buffer allows you to interact with your existing workflow to solve that problem.

However, you can have the best of both worlds by using another callback function, that way people can choose whatever they want when selecting attachments.
I.e. when in doubt, use a callback

@jugarpeupv
Copy link
Copy Markdown
Contributor

@yousefakbar is it possible to merge this PR? at least we have a working way to include attachments, there is always room to improvements or support other approach like @simonhughxyz was referencing, which has its point as well

@yousefakbar
Copy link
Copy Markdown
Owner Author

@yousefakbar is it possible to merge this PR? at least we have a working way to include attachments, there is always room to improvements or support other approach like @simonhughxyz was referencing, which has its point as well

Apologies I'm currently travelling so I may not be as fast with my responses or work here :-).

I will be taking @simonhughxyz 's feedback into consideration and having options for both.

In the meantime, the original implementation from #7 is still there and you can use the <C-g><C-a> binding to manage and attachments in composing emails.

@jugarpeupv
Copy link
Copy Markdown
Contributor

@yousefakbar I just implemented an approach based on a neovim buffer, it works with oil as well, if you copy files from oil.nvim buffers and paste it into attachment buffer it works

Stil the user can add attachments by invoking Attach in case they do not use oil.nvim

Now it is quite convenient, hope you find it useful as well

#39

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.

3 participants