Directly piping files into external commands (v2)#291
Open
bptato wants to merge 2 commits intotats:masterfrom
Open
Directly piping files into external commands (v2)#291bptato wants to merge 2 commits intotats:masterfrom
bptato wants to merge 2 commits intotats:masterfrom
Conversation
Mailcap entries that * have no copiousoutput flag and * have no x-htmloutput flag and * have no %s markers are no longer saved to the file system before being piped into the mailcap command.
Contributor
Author
|
Sorry, I messed up a few things; save2tmp was being called twice sometimes + |
a7db4fd to
acf8914
Compare
Contributor
I don't understand this. AFAIS the only difference between UFclose and UFhalfclose is, that the latter decides based on the scheme which close function to call. With any scheme other than SCM_FTP or SCM_NEWS/_NNTP is will call UFclose. |
Contributor
Author
|
Rene Kita wrote:
I don't understand this. AFAIS the only difference between UFclose
and UFhalfclose is, that the latter decides based on the scheme which
close function to call. With any scheme other than SCM_FTP or
SCM_NEWS/_NNTP is will call UFclose.
Indeed. My understanding is that for the other stream types, UFclose
just closes a FILE. On the other hand, FTP's UFclose waits for the
server to respond through closeFTPdata, so that would get the browser
stuck until the download is finished.
You can see the same pattern of UFhalfclose used to avoid this problem
in doFileSave.
(Admittedly, I haven't tested it with NEWS/NNTP. I had assumed the
mechanism is similar, and indeed, there is a news_quit function in
news.c that talks to the server. However, the news->rf InputStream
doesn't seem to override basic_close, so I don't really see what is
going on there.)
|
Contributor
|
On Wed, Jul 31, 2024 at 11:11:45AM -0700, bptato wrote:
Rene Kita wrote:
> I don't understand this. AFAIS the only difference between UFclose
> and UFhalfclose is, that the latter decides based on the scheme which
> close function to call. With any scheme other than SCM_FTP or
> SCM_NEWS/_NNTP is will call UFclose.
Indeed. My understanding is that for the other stream types, UFclose
just closes a FILE. On the other hand, FTP's UFclose waits for the
server to respond through closeFTPdata, so that would get the browser
stuck until the download is finished.
You can see the same pattern of UFhalfclose used to avoid this problem
in doFileSave.
Interesting, I will need to have a closer look. This has some smell. I'd
say we could refactor the code so that your need_halfclose is not needed
anymore. This is probably out of scope for this PR, though.
… (Admittedly, I haven't tested it with NEWS/NNTP. I had assumed the
mechanism is similar, and indeed, there is a news_quit function in
news.c that talks to the server. However, the news->rf InputStream
doesn't seem to override basic_close, so I don't really see what is
going on there.)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Includes the following:
This is a simpler patch than my previous attempt1.
PIPE_LINKandx-nosaveinputhave been dropped. Instead, streaming is enabled for thecases where:
%stemplate exists in the mailcap entry, andAnother improvement over the previous patch: UFhalfclose is used instead
of UFclose, so that the parent process does not wait for background external
viewer processes to finish downloading.
Stuff not implemented:
this would be the "right thing" to do, it seems difficult to implement +
pointless since w3m does not support incremental rendering anyway. So I
decided to stick with the "download first" approach for these cases.
before the entry is executed, but in the end I decided to keep the scope
of this PR narrower and did not add it. Maybe in a future PR.
Footnotes
https://github.com/tats/w3m/pull/275 ↩