mirror of
https://github.com/fish-shell/fish-shell
synced 2025-01-01 15:48:45 +00:00
Allow eval to see the tty if its output is not piped
Commit 5fccfd83ec
, with the fix for #6806,
switched eval to buffer its output (like other builtins do). But this
prevents using eval with commands that wants to see the tty, especially
fzf. So only buffer the output if the output is piped to the next process.
This commit is contained in:
parent
f3d31f6ef6
commit
eabe2e8855
1 changed files with 40 additions and 21 deletions
|
@ -27,24 +27,40 @@ int builtin_eval(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
|
||||||
new_cmd += argv[i];
|
new_cmd += argv[i];
|
||||||
}
|
}
|
||||||
|
|
||||||
// The output and errput of eval must go to our streams, not to the io_chain in our streams,
|
// If stdout is piped, then its output must go to the streams, not to the io_chain in our
|
||||||
// because they may contain a pipe which is intended to be consumed by a process which is not
|
// streams, because the pipe may be intended to be consumed by a process which
|
||||||
// yet launched (#6806). Make bufferfills to capture it.
|
// is not yet launched (#6806). If stdout is NOT redirected, it must see the tty (#6955). So
|
||||||
// TODO: we are sloppy and do not honor other redirections; eval'd code won't see for example a
|
// create a bufferfill for stdout if and only if stdout is piped.
|
||||||
// redirection of fd 3.
|
// Note do not do this if stdout is merely redirected (say, to a file); we don't want to
|
||||||
shared_ptr<io_bufferfill_t> stdout_fill =
|
// buffer in that case.
|
||||||
io_bufferfill_t::create(fd_set_t{}, parser.libdata().read_limit, STDOUT_FILENO);
|
shared_ptr<io_bufferfill_t> stdout_fill{};
|
||||||
shared_ptr<io_bufferfill_t> stderr_fill =
|
if (streams.out_is_piped) {
|
||||||
io_bufferfill_t::create(fd_set_t{}, parser.libdata().read_limit, STDERR_FILENO);
|
stdout_fill =
|
||||||
if (!stdout_fill || !stderr_fill) {
|
io_bufferfill_t::create(fd_set_t{}, parser.libdata().read_limit, STDOUT_FILENO);
|
||||||
// This can happen if we are unable to create a pipe.
|
if (!stdout_fill) {
|
||||||
return STATUS_CMD_ERROR;
|
// We were unable to create a pipe, probably fd exhaustion.
|
||||||
|
return STATUS_CMD_ERROR;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Of course the same applies to stderr.
|
||||||
|
shared_ptr<io_bufferfill_t> stderr_fill{};
|
||||||
|
if (streams.err_is_piped) {
|
||||||
|
stderr_fill =
|
||||||
|
io_bufferfill_t::create(fd_set_t{}, parser.libdata().read_limit, STDERR_FILENO);
|
||||||
|
if (!stderr_fill) {
|
||||||
|
return STATUS_CMD_ERROR;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Construct the full io chain, perhaps with our bufferfills appended.
|
||||||
|
io_chain_t ios = *streams.io_chain;
|
||||||
|
if (stdout_fill) ios.push_back(stdout_fill);
|
||||||
|
if (stderr_fill) ios.push_back(stderr_fill);
|
||||||
|
|
||||||
const auto cached_exec_count = parser.libdata().exec_count;
|
const auto cached_exec_count = parser.libdata().exec_count;
|
||||||
int status = STATUS_CMD_OK;
|
int status = STATUS_CMD_OK;
|
||||||
if (parser.eval(new_cmd, io_chain_t{stdout_fill, stderr_fill}, block_type_t::top,
|
if (parser.eval(new_cmd, ios, block_type_t::top, streams.parent_pgid) != eval_result_t::ok) {
|
||||||
streams.parent_pgid) != eval_result_t::ok) {
|
|
||||||
status = STATUS_CMD_ERROR;
|
status = STATUS_CMD_ERROR;
|
||||||
} else if (cached_exec_count == parser.libdata().exec_count) {
|
} else if (cached_exec_count == parser.libdata().exec_count) {
|
||||||
// Issue #5692, in particular, to catch `eval ""`, `eval "begin; end;"`, etc.
|
// Issue #5692, in particular, to catch `eval ""`, `eval "begin; end;"`, etc.
|
||||||
|
@ -55,14 +71,17 @@ int builtin_eval(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Finish the bufferfills - exhaust and close our pipes.
|
// Finish the bufferfills - exhaust and close our pipes.
|
||||||
|
// Copy the output from the bufferfill back to the streams.
|
||||||
// Note it is important that we hold no other references to the bufferfills here - they need to
|
// Note it is important that we hold no other references to the bufferfills here - they need to
|
||||||
// deallocate to close.
|
// deallocate to close.
|
||||||
std::shared_ptr<io_buffer_t> output = io_bufferfill_t::finish(std::move(stdout_fill));
|
ios.clear();
|
||||||
std::shared_ptr<io_buffer_t> errput = io_bufferfill_t::finish(std::move(stderr_fill));
|
if (stdout_fill) {
|
||||||
|
std::shared_ptr<io_buffer_t> output = io_bufferfill_t::finish(std::move(stdout_fill));
|
||||||
// Copy the output from the bufferfill back to the streams.
|
streams.out.append_narrow_buffer(output->buffer());
|
||||||
streams.out.append_narrow_buffer(output->buffer());
|
}
|
||||||
streams.err.append_narrow_buffer(errput->buffer());
|
if (stderr_fill) {
|
||||||
|
std::shared_ptr<io_buffer_t> errput = io_bufferfill_t::finish(std::move(stderr_fill));
|
||||||
|
streams.err.append_narrow_buffer(errput->buffer());
|
||||||
|
}
|
||||||
return status;
|
return status;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue