Skip to content

Conversation

@Konfekt
Copy link
Contributor

@Konfekt Konfekt commented Jun 16, 2018

No description provided.

@Konfekt
Copy link
Contributor Author

Konfekt commented Jun 20, 2018

Somehow this command is not triggered by a BufWrite trigger.

@chrisbra
Copy link
Owner

Why do you need this?

@Konfekt
Copy link
Contributor Author

Konfekt commented Jun 21, 2018

Because every line Attach: file with unescaped space in path is removed and the file not attached.

@Konfekt
Copy link
Contributor Author

Konfekt commented Jun 21, 2018

If the question referred to the BufWrite trigger, it is a saner alternative to the BufWriteCmd trigger, see #22 (comment) .

Indeed, the BufWrite event does not work because CheckAttach uses a BufWriteCmd trigger.

@chrisbra
Copy link
Owner

I don't understand. For me, it always escapes spaces. And that is what it actually should:

Here for the automatic check on saving:

call append(s:header_end, 'Attach: ' .
\ escape(fnamemodify(attach, ':p'), " \t\\"))

Here when using an external file browser:

call append('.', map(readfile(s:external_choosefile), '"Attach: ".
\ escape(v:val, " \t\\")'))

Here when using :AttachFile:

call append('.', 'Attach: '. escape(fnamemodify(item, ':p'), " \t\\"))

So when do you exactly see that problem?

@Konfekt
Copy link
Contributor Author

Konfekt commented Jun 24, 2018

Yes, CheckAttach escapes spaces when a file is appended by the :AttachFile command, but not when entered directly. That is, for example, after starting a new line with Attach: and copy-pasting the file name obtained elsewhere.

@Konfekt
Copy link
Contributor Author

Konfekt commented Jun 24, 2018

Well, regarding

call append(s:header_end, 'Attach: ' .
\ escape(fnamemodify(attach, ':p'), " \t\\"))

I am lost, it didn't escape spaces over here.

@Konfekt
Copy link
Contributor Author

Konfekt commented Jun 24, 2018

Well, perhaps use fnameescape() instead of escape(fnamemodify()) ?

@chrisbra
Copy link
Owner

I think the script has been created, when fnameescape() wasn't even available yet. BTW: while you can of course manually adding the :Attach header, i would not recommend it, because this depends on your $PWD and you would need to manually escape strings as you found out.

Regarding:

I am lost, it didn't escape spaces over here.

Can you step through the function and check why it doesn't work for you? You can manually start debugging by running :debug :w and then follow the instructions at :h debug-scripts.

@Konfekt
Copy link
Contributor Author

Konfekt commented Jun 25, 2018

you would need to manually escape strings as you found out.

There was a misunderstanding. I thought

call append(s:header_end, 'Attach: ' .
\ escape(fnamemodify(attach, ':p'), " \t\\"))

is there to automatize the escaping when saving or quitting Vim. But it seems to me that

 let ans = input(prompt, "", "file")
    while (ans != '') && (ans != 'n')
      if empty(s:external_file_browser)
let list = split(expand(ans), "\n")

only escapes file names attached by the CheckAttach input dialogue. This pull request is about escaping all files that were attached, regardless if by the CheckAttach input dialogue or other means (in my case, copy pasting the full path (not using the $PWD)).

@chrisbra
Copy link
Owner

so you are manually adding the pseudo attach header and want the plugin to escape the filename automatically on save?

@Konfekt
Copy link
Contributor Author

Konfekt commented Jul 14, 2018

That's it, I think. I frequently paste file paths from a file manager, such as Krusader. If these aren't escaped, they disappear on entering the send dialog of mutt.

@chrisbra chrisbra closed this in 0f1f2e7 Jul 18, 2018
@chrisbra
Copy link
Owner

Please check current head. I made some changes to your patch and implemented it slightly different. The result should work however. Note, I intentionally did not use fnameescape() because I think it might escape too much. Also I wasn't sure, what the system("test -r" call was there for in the first place. So I omitted that. It seems to be superfluous and too expansive anyhow.

@Konfekt
Copy link
Contributor Author

Konfekt commented Jul 18, 2018

I wasn't sure, what the system("test -r" call was there for

This checks if the file path is already escaped or not. If the system already recognizes the file, then it must not be escaped.

I intentionally did not use fnameescape() because I think it might escape too much

I am not sure why fnameescape() should escape too much?

@chrisbra
Copy link
Owner

I am not sure why fnameescape() should escape too much?

It escapes things such as '%' or '#' for use by e.g. internal Vim commands, that would otherwise expand e.g. '%' to the currently edited file. However, I think those shouldn't be escaped, or else mutt won't find it.

Konfekt referenced this pull request in EinfachToll/DidYouMean Oct 31, 2018
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.

2 participants