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

[RDY] Remove "reading from stdin" message #6298

Merged
merged 1 commit into from Mar 18, 2017

Conversation

Projects
None yet
8 participants
@msva
Contributor

msva commented Mar 16, 2017

How about finally (after all that years, tons of emails in the mailing lists and messages on the IRC) give users possibility to suppress that noisy message, if they know what and why they are doing (i.e. set env variable)

P.S. Variable is named VIM_* to interoperability with original vim (PR#1564), and do not force users, who use both of them to set tons of identical variables for the same thing.
// Although, it is discussable...

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Mar 16, 2017

Neovim has --headless which can be used to not start the TUI and -s to still read input: nvim --headless -s /dev/stdin should do what you need.

@msva

This comment has been minimized.

Contributor

msva commented Mar 16, 2017

Although, I'll try to re-work that to just obey -V0

@msva msva changed the title from [RFC] Give user possibility to suppress "reading from stdin" message to [WIP] Give user possibility to suppress "reading from stdin" message Mar 16, 2017

@msva

This comment has been minimized.

Contributor

msva commented Mar 16, 2017

@ZyX-I it can be useful to do start a TUI, but do not print this message (like, say, nvimpager).
And anyway, passing /dev/stdin looks like a hack.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Mar 16, 2017

Regardless of your use-case it is probably good idea to special-case -s - to read from stdin, just need to have something like

diff --git a/src/nvim/main.c b/src/nvim/main.c
index fdcf636..d2cfcf7 100644
--- a/src/nvim/main.c
+++ b/src/nvim/main.c
@@ -1054,7 +1054,9 @@ scripterror:
               mch_errmsg("\"\n");
               mch_exit(2);
             }
-            if ((scriptin[0] = mch_fopen(argv[0], READBIN)) == NULL) {
+            if (STRCMP(argv[0], "-") == 0) {
+              scriptin[0] = stdin;
+            } else if ((scriptin[0] = mch_fopen(argv[0], READBIN)) == NULL) {
               mch_errmsg(_("Cannot open for reading: \""));
               mch_errmsg(argv[0]);
               mch_errmsg("\"\n");

if I am not mistaking. Not all systems have /dev/stdin and using - for stdin is a common convention.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Mar 16, 2017

And for vim/vim#1552: you do not need -s, but why do you need TUI? Though I am wrong: nvim --headless - still gives the message, and it gives it to stdout, -s - is for other use-cases.

@marvim marvim added the WIP label Mar 16, 2017

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Mar 16, 2017

-V0 is not logical: it by definition should set &verbose option to 0 and verbose 0 is default verbose option value. As you see at verbose level 0 this message is shown. So this should be something else: --silent?

Also why I get E484: Can't open file /home/zyx/a.a/Proj/c/neovim/build/root/share/nvim/syntax/syntax.vim to stderr and Nvim: Reading from stdin to stdout? Even nvim --headless --cmd 'echo 42' will print to stderr! This is a good point in vim/vim#1552 (comment).

@msva

This comment has been minimized.

Contributor

msva commented Mar 16, 2017

By the way, Bram suggested to make option --read-stdin, which:

Or just --read-stdin, which would do the same as "-" but not give the message.

And, I like this way. Any objections? :)

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Mar 16, 2017

It is still inconsistent: almost any message from headless instance goes to stderr, but this message goes to stdout. As per vim/vim#1552 (comment) this will fix the case just as well, but will not require any new command-line options.

Otherwise if there are or may be in the future any other messages printed to stdout it is better to have catch-all --silent (or --silent-stdout?) which is guaranteed to suppress any *vim output that otherwise goes to stdout rather then have 100500 suppression options, one per message. (Alt: --stderr-messages.)

I.e. if message you are dealing with is unique then it is inconsistent and should go to stderr, no options are needed. If message is not unique, add a flag to suppress all stdout messages.

--read-stdin suppressing stderr message in the first case is fine though.

@justinmk

This comment has been minimized.

Member

justinmk commented Mar 17, 2017

Why is the message needed at all. It's useless. And more options sounds bad.

@msva

This comment has been minimized.

Contributor

msva commented Mar 17, 2017

@justinmk

This comment has been minimized.

Member

justinmk commented Mar 17, 2017

I know the reason, I think it is not a good reason.

Let's just remove the message completely. The message is addressing the uncommon case, and it makes the common case unpleasant.

@msva msva force-pushed the msva:bye-stdin-warn branch from f2bcf4c to 7e9ac1d Mar 17, 2017

@msva msva changed the title from [WIP] Give user possibility to suppress "reading from stdin" message to [RFC] Give user possibility to suppress "reading from stdin" message Mar 17, 2017

@marvim marvim added RFC and removed WIP labels Mar 17, 2017

@msva

This comment has been minimized.

Contributor

msva commented Mar 17, 2017

Well, I did it in same way, as I did it for vim itself.

@justinmk, do you recommend to just drop that warning at all in this PR?

@justinmk

This comment has been minimized.

Member

justinmk commented Mar 17, 2017

Yes, I think we should drop it. It is an unusual behavior for command-line tools. Non-experienced users are unlikely to encounter the problem. Experienced users already expect the behavior.

It doesn't make sense that so many things in Vim behave with the expectation that users study a giant menu of options, but for this singular case Vim is desperate to avoid any complaints from anyone ever.

And adding more and more options is a cop-out.

@msva msva force-pushed the msva:bye-stdin-warn branch from fdbf3a9 to 9c4b1e4 Mar 17, 2017

@msva

This comment has been minimized.

Contributor

msva commented Mar 17, 2017

Done

filemess(curbuf, sfname, (char_u *)"", 0);
}

This comment has been minimized.

@justinmk

justinmk Mar 17, 2017

Member

Can you explain why the check for !read_buffer was added?

This comment has been minimized.

@msva

msva Mar 17, 2017

Contributor

I don't know why it was added before, but I just made condition, which will be triggered in same conditions as before.

There was "if read stdin then throw warning, otherwise, if not read buffer, then filemess".
I did "if not read stdin and not read buffer, then filemess".

So, filemess will be triggered in exact conditions as before: when both (!read_stdin) and (!read_buffer)...

This comment has been minimized.

@justinmk

justinmk Mar 18, 2017

Member

I somehow missed it in the diff. My mistake.

@@ -706,10 +706,9 @@ readfile (
* still be running, don't move the cursor to the last line, unless
* always using the GUI.

This comment has been minimized.

@justinmk

justinmk Mar 17, 2017

Member

that whole comment can be deleted.

This comment has been minimized.

@msva

msva Mar 17, 2017

Contributor

ok

@msva msva force-pushed the msva:bye-stdin-warn branch from 9c4b1e4 to 041d572 Mar 17, 2017

if (read_stdin) {
mch_msg(_("Nvim: Reading from stdin...\n"));
} else if (!read_buffer)
if ((!read_stdin) && (!read_buffer)) {

This comment has been minimized.

@justinmk

justinmk Mar 18, 2017

Member

remove the extraneous parens, that isn't common in this project

This comment has been minimized.

@msva

msva Mar 18, 2017

Contributor

done

@msva msva changed the title from [RFC] Give user possibility to suppress "reading from stdin" message to [RDY] Give user possibility to suppress "reading from stdin" message Mar 18, 2017

@msva msva changed the title from [RDY] Give user possibility to suppress "reading from stdin" message to [RDY] Dropping out "reading from stdin" message Mar 18, 2017

@msva msva force-pushed the msva:bye-stdin-warn branch from 041d572 to b96e8a5 Mar 18, 2017

@msva msva force-pushed the msva:bye-stdin-warn branch from b96e8a5 to cd17958 Mar 18, 2017

@justinmk justinmk merged commit bdcb2a3 into neovim:master Mar 18, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@justinmk justinmk removed the RFC label Mar 18, 2017

@justinmk justinmk changed the title from [RDY] Dropping out "reading from stdin" message to [RDY] Remove "reading from stdin" message Mar 18, 2017

@msva msva deleted the msva:bye-stdin-warn branch Mar 18, 2017

@msva

This comment has been minimized.

Contributor

msva commented Apr 8, 2017

justinmk added a commit to justinmk/neovim that referenced this pull request May 1, 2017

NVIM v0.2.0
FEATURES:
    bc4a2e1 help, man.vim: "outline" (TOC) feature neovim#5169
    58422f1 'guicursor' works in the TUI (and sends info to UIs) neovim#6423
    129f107 api: nvim_get_mode() neovim#6247
    0b59f98 api/ui: externalize tabline neovim#6583
    bc6d868 'listchars': `Whitespace` highlight group neovim#6367
    6afa7d6 writefile() obeys 'fsync' option neovim#6427
    c60e409 eval.c refactor (also improves some error messages) neovim#5119
    9d200cd getcompletion("cmdline") neovim#6376
    2ea7bfc terminal: Support extra arguments in 'shell'. neovim#4504
    bf51102 DirChanged autocmd neovim#5928 neovim#6262
    1743df8 'cpoptions': "_" flag to toggle `cw` behaviour neovim#6235
    22337b1 CTRL-R omits trailing ^M when pasting to cmdline neovim#6137
    0e44916 :edit allows unescaped spaces in filename neovim#6119
    abdbfd2 eval: Add id() function and make printf("%p") useful neovim#6095
    bdfa147 findfile(), :find, gf work in :terminal. neovim#6009
    2f38ed1 providers: Disable if `g:loaded_*` exists.
    b5560a6 setpos() can set lowercase marks in other buffers neovim#5753
    7c513d6 Throttle :! output, pulse "..." message. neovim#5396
    d2e8c76 v:exiting neovim#5651

    :terminal improvements neovim#6185 neovim#6142
      - cursor keeps position after leaving insert-mode.
      - 4ceec30 Follows output only if cursor is at end of buffer.
      - e7bbd35 new option: 'scrollback'
      - fedb844 quasi-support for undo and 'modifiable'
      - b45ddf7 disables 'list' by default
      - disables 'relativenumber' by default

    :help now contains full API documentation at `:help api`.

    man.vim saw numerous improvements.

    Windows support:
      - Windows is no longer "experimental", it is fully supported.
      - Windows package includes a GUI, curl.exe and other utilities.

    "Vim 8" features: partials, lambdas, packages.

FIXES:
    12fc1de ops: fix i<c-r> with multi-byte text neovim#6524
    dd391bf Windows: system() and friends neovim#6497
    13352c0 Windows: os_get_hostname() neovim#6413
    16babc6 tui: Less-noisy mouse seqs neovim#6411
    3a9dd13 (vim bug) folding edge-cases  neovim#6207
    f6946c6 job-control: set CLOEXEC on pty processes. neovim#5986
    d1afd43 rplugin: Call s:LoadRemotePlugins() on startup.
    1215084 backtick-expansion works with `shell=fish` neovim#6224
    e32ec03 tui: Improved behavior after resize. neovim#6202
    86c2adc edit.c: CTRL-SPC: Insert previously-inserted text. neovim#6090
    c318d8e b:changedtick now follows VimL rules neovim#6112
    34e24cb terminal: Initialize colors in reverse order neovim#6160
    e889917 undo: Don't set b_u_curhead in ex_undojoin() neovim#5869
    d25649f undo: :earlier, g-: Set b_u_seq_cur correctly. (neovim#6016)
    043d8ba 'Visual-mode put from @. register' neovim#5782
    42c922b open_buffer(): Do `BufEnter` for directories.
    50d0d89 inccommand: Preview :sub commands only after delimiter neovim#5932
    1420e10 CheckHealth improvements neovim#5519
    c8d5e92 jobstart(): Return -1 if cmd is not executable. neovim#5671

CHANGES:
    NVIM_TUI_ENABLE_CURSOR_SHAPE was removed. Use 'guicursor' instead.
        See https://github.com/neovim/neovim/wiki/Following-HEAD#20170402

    81525dc 'mouse=a' is no longer the default. (This will probably
                 change again after it is improved.) neovim#6022

    0c1f783 defaults: 'showcmd', 'belloff', 'ruler' neovim#6087
    eb0e94f api: {get,set}_option update local options as appropriate neovim#6405
    bdcb2a3 "Reading from stdin..." message was removed. neovim#6298

justinmk added a commit to justinmk/neovim that referenced this pull request May 1, 2017

NVIM v0.2.0
FEATURES:
    bc4a2e1 help, man.vim: "outline" (TOC) feature neovim#5169
    58422f1 'guicursor' works in the TUI (and sends info to UIs) neovim#6423
    129f107 api: nvim_get_mode() neovim#6247
    0b59f98 api/ui: externalize tabline neovim#6583
    bc6d868 'listchars': `Whitespace` highlight group neovim#6367
    6afa7d6 writefile() obeys 'fsync' option neovim#6427
    c60e409 eval.c refactor (also improves some error messages) neovim#5119
    9d200cd getcompletion("cmdline") neovim#6376
    2ea7bfc terminal: Support extra arguments in 'shell'. neovim#4504
    bf51102 DirChanged autocmd neovim#5928 neovim#6262
    1743df8 'cpoptions': "_" flag to toggle `cw` behaviour neovim#6235
    22337b1 CTRL-R omits trailing ^M when pasting to cmdline neovim#6137
    0e44916 :edit allows unescaped spaces in filename neovim#6119
    abdbfd2 eval: Add id() function and make printf("%p") useful neovim#6095
    bdfa147 findfile(), :find, gf work in :terminal. neovim#6009
    2f38ed1 providers: Disable if `g:loaded_*` exists.
    b5560a6 setpos() can set lowercase marks in other buffers neovim#5753
    7c513d6 Throttle :! output, pulse "..." message. neovim#5396
    d2e8c76 v:exiting neovim#5651

    :terminal improvements neovim#6185 neovim#6142
      - cursor keeps position after leaving insert-mode.
      - 4ceec30 Follows output only if cursor is at end of buffer.
      - e7bbd35 new option: 'scrollback'
      - fedb844 quasi-support for undo and 'modifiable'
      - b45ddf7 disables 'list' by default
      - disables 'relativenumber' by default

    :help now contains full API documentation at `:help api`.

    man.vim saw numerous improvements.

    Windows support:
      - Windows is no longer "experimental", it is fully supported.
      - Windows package includes a GUI, curl.exe and other utilities.

    "Vim 8" features: partials, lambdas.

SECURITY FIXES:
    CVE-2017-5953 CVE-2017-6349 CVE-2017-6350 neovim#6485

CHANGES:
    NVIM_TUI_ENABLE_CURSOR_SHAPE was removed. Use 'guicursor' instead.
        See https://github.com/neovim/neovim/wiki/Following-HEAD#20170402

    81525dc 'mouse=a' is no longer the default. (This will probably
                 change again after it is improved.) neovim#6022

    0c1f783 defaults: 'showcmd', 'belloff', 'ruler' neovim#6087
    eb0e94f api: {get,set}_option update local options as appropriate neovim#6405
    bdcb2a3 "Reading from stdin..." message was removed. neovim#6298

FIXES:
    12fc1de ops: fix i<c-r> with multi-byte text neovim#6524
    dd391bf Windows: system() and friends neovim#6497
    13352c0 Windows: os_get_hostname() neovim#6413
    16babc6 tui: Less-noisy mouse seqs neovim#6411
    3a9dd13 (vim bug) folding edge-cases  neovim#6207
    f6946c6 job-control: set CLOEXEC on pty processes. neovim#5986
    d1afd43 rplugin: Call s:LoadRemotePlugins() on startup.
    1215084 backtick-expansion works with `shell=fish` neovim#6224
    e32ec03 tui: Improved behavior after resize. neovim#6202
    86c2adc edit.c: CTRL-SPC: Insert previously-inserted text. neovim#6090
    c318d8e b:changedtick now follows VimL rules neovim#6112
    34e24cb terminal: Initialize colors in reverse order neovim#6160
    e889917 undo: Don't set b_u_curhead in ex_undojoin() neovim#5869
    d25649f undo: :earlier, g-: Set b_u_seq_cur correctly. (neovim#6016)
    043d8ba 'Visual-mode put from @. register' neovim#5782
    42c922b open_buffer(): Do `BufEnter` for directories.
    50d0d89 inccommand: Preview :sub commands only after delimiter neovim#5932
    1420e10 CheckHealth improvements neovim#5519
    c8d5e92 jobstart(): Return -1 if cmd is not executable. neovim#5671

justinmk added a commit to justinmk/neovim that referenced this pull request May 1, 2017

NVIM v0.2.0
FEATURES:
    bc4a2e1 help, man.vim: "outline" (TOC) feature neovim#5169
    58422f1 'guicursor' works in the TUI (and sends info to UIs) neovim#6423
    129f107 api: nvim_get_mode() neovim#6247
    0b59f98 api/ui: externalize tabline neovim#6583
    bc6d868 'listchars': `Whitespace` highlight group neovim#6367
    6afa7d6 writefile() obeys 'fsync' option neovim#6427
    c60e409 eval.c refactor (also improves some error messages) neovim#5119
    9d200cd getcompletion("cmdline") neovim#6376
    2ea7bfc terminal: Support extra arguments in 'shell'. neovim#4504
    bf51102 DirChanged autocmd neovim#5928 neovim#6262
    1743df8 'cpoptions': "_" flag to toggle `cw` behaviour neovim#6235
    22337b1 CTRL-R omits trailing ^M when pasting to cmdline neovim#6137
    0e44916 :edit allows unescaped spaces in filename neovim#6119
    abdbfd2 eval: Add id() function and make printf("%p") useful neovim#6095
    bdfa147 findfile(), :find, gf work in :terminal. neovim#6009
    2f38ed1 providers: Disable if `g:loaded_*` exists.
    b5560a6 setpos() can set lowercase marks in other buffers neovim#5753
    7c513d6 Throttle :! output, pulse "..." message. neovim#5396
    d2e8c76 v:exiting neovim#5651

    :terminal improvements neovim#6185 neovim#6142
      - cursor keeps position after leaving insert-mode.
      - 4ceec30 Follows output only if cursor is at end of buffer.
      - e7bbd35 new option: 'scrollback'
      - fedb844 quasi-support for undo and 'modifiable'
      - b45ddf7 disables 'list' by default
      - disables 'relativenumber' by default

    :help now contains full API documentation at `:help api`.

    man.vim saw numerous improvements.

    Windows support:
      - Windows is no longer "experimental", it is fully supported.
      - Windows package includes a GUI, curl.exe and other utilities.

    "Vim 8" features: partials, lambdas.

SECURITY FIXES:
    CVE-2017-5953 CVE-2017-6349 CVE-2017-6350 neovim#6485

CHANGES:
    NVIM_TUI_ENABLE_CURSOR_SHAPE was removed. Use 'guicursor' instead.
        See https://github.com/neovim/neovim/wiki/Following-HEAD#20170402

    81525dc 'mouse=a' is no longer the default. (This will probably
                 change again after it is improved.) neovim#6022

    0c1f783 defaults: 'showcmd', 'belloff', 'ruler' neovim#6087
    eb0e94f api: {get,set}_option update local options as appropriate neovim#6405
    bdcb2a3 "Reading from stdin..." message was removed. neovim#6298

FIXES:
    12fc1de ops: fix i<c-r> with multi-byte text neovim#6524
    dd391bf Windows: system() and friends neovim#6497
    13352c0 Windows: os_get_hostname() neovim#6413
    16babc6 tui: Less-noisy mouse seqs neovim#6411
    3a9dd13 (vim bug) folding edge-cases  neovim#6207
    f6946c6 job-control: set CLOEXEC on pty processes. neovim#5986
    d1afd43 rplugin: Call s:LoadRemotePlugins() on startup.
    1215084 backtick-expansion works with `shell=fish` neovim#6224
    e32ec03 tui: Improved behavior after resize. neovim#6202
    86c2adc edit.c: CTRL-SPC: Insert previously-inserted text. neovim#6090
    c318d8e b:changedtick now follows VimL rules neovim#6112
    34e24cb terminal: Initialize colors in reverse order neovim#6160
    e889917 undo: Don't set b_u_curhead in ex_undojoin() neovim#5869
    d25649f undo: :earlier, g-: Set b_u_seq_cur correctly. (neovim#6016)
    043d8ba 'Visual-mode put from @. register' neovim#5782
    42c922b open_buffer(): Do `BufEnter` for directories.
    50d0d89 inccommand: Preview :sub commands only after delimiter neovim#5932
    1420e10 CheckHealth improvements neovim#5519
    c8d5e92 jobstart(): Return -1 if cmd is not executable. neovim#5671
@rdebath

This comment has been minimized.

rdebath commented Sep 3, 2017

I'm not sure how dumb I'm being, but isn't the purpose of this message to tell some "luser" that they have to type something when they enter a bare vim - command.
That is:

  1. The message should only show if the user hasn't redirected stdin
$ gunzip -
gzip: compressed data not read from a terminal.
  1. The message should probably be more descriptive.
$ vim -
Vim: Reading from terminal, enter text then press Ctrl-D...

So this patch would do enough.

diff --git a/src/fileio.c b/src/fileio.c
index 87a46b1..939403b 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -825,7 +825,7 @@ readfile(
         * still be running, don't move the cursor to the last line, unless
         * always using the GUI.
         */
-       if (read_stdin)
+       if (read_stdin && isatty(0))
        {
 #ifndef ALWAYS_USE_GUI
            mch_msg(_("Vim: Reading from stdin...\n"));

PS: Oops, wrong vim. Yup, that's a bit dumb 😁

@kenorb

This comment has been minimized.

kenorb commented Dec 13, 2017

This has been addressed in Vim in 234d162 (as per #1552), so it won't display the message when --not-a-term is specified.

@justinmk

This comment has been minimized.

Member

justinmk commented Dec 13, 2017

As usual, that doesn't really address anything, it dodges the issue and makes the user decide.

@cup

This comment has been minimized.

cup commented Dec 13, 2017

@justinmk yeah - its not ideal - i would prefer a short option like -n or similar - also with proper logic a switch shouldnt be needed at all - but its better than nothing

@chrisbra

This comment has been minimized.

chrisbra commented Dec 14, 2017

As usual, that doesn't really address anything, it dodges the issue and makes the user decide.

It is a good compromise. Users unaware of the issue get a warning, others can skip it entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment