Skip to content
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

weird: banner && warn("Terminal not fully functional") #23380

Closed
StefanKarpinski opened this issue Aug 21, 2017 · 17 comments
Closed

weird: banner && warn("Terminal not fully functional") #23380

StefanKarpinski opened this issue Aug 21, 2017 · 17 comments
Labels
display and printing Aesthetics and correctness of printed representations of objects. good first issue Indicates a good issue for first-time contributors to Julia help wanted Indicates that a maintainer wants help on an issue or pull request REPL Julia's REPL (Read Eval Print Loop)

Comments

@StefanKarpinski
Copy link
Member

I'm not sure what to title this (hence the bad title), but as observed in https://github.com/JuliaLang/julia/pull/23343/files#r134114620, it makes very little sense that an option that otherwise only controls whether we print a banner or not gates whether we print a "Terminal not fully functional" message or not. This made a little sense when the option was called quiet but in practice it was only used to suppress banner printing and we don't use that option to suppress warning output anywhere else.

@StefanKarpinski StefanKarpinski added display and printing Aesthetics and correctness of printed representations of objects. REPL Julia's REPL (Read Eval Print Loop) labels Aug 21, 2017
@yuyichao
Copy link
Contributor

It does seem to control all REPL warnings?

@StefanKarpinski
Copy link
Member Author

Links to other instances? If that's the case, we should reintroduce -q to suppress REPL warnings in addition to turning off the banner and document it to do so, but keep --banner={yes|no} to control just the banner printing. So you could potentially suppress warnings but keep the banner by doing julia -q --banner=yes.

@yuyichao
Copy link
Contributor

Links to other instances?

I said that since I didn't find any (in fact, the existence of other instance not covered in the pr will be a proof that the option is not used to suppress repl output).

So it does seem like quiet is a more correct name in that it disable repl messages, not just banner, and the change should be reverted (or at least it shouldn't be called banner, maybe quiet repl.) Banner is also weird since IIUC it does not enable banner?

@StefanKarpinski
Copy link
Member Author

So you're arguing that because there is only one REPL warning and it was controlled by this option, even though --quiet was only documented to suppress banner printing, we should consider --quiet to suppress all REPL warnings? That would be fine, and as I said, we could reintroduce --quiet explicitly as a banner and warning-suppressing option.

That doesn't lessen the need for a banner-only option, which is what --quiet was supposed to do according to the docs anyway. We don't do double negatives in options, so the appropriate name for that option is --banner with a required argument, defaulting to yes, which is exactly what @HarrisonGrodin implemented.

@StefanKarpinski
Copy link
Member Author

I also don't think that --quiet is a very good name for "suppress REPL warnings". That name would suggest suppressing all output or something, not just warnings.

@yuyichao
Copy link
Contributor

even though --quiet was only documented to suppress banner printing

Actually not really. The original doc says Quiet startup (no banner), which is pretty accurate even though the explanation in the paranthesis can be confusing so I think what it need is just a clarification that this option is to give you a "quiet startup" which includes no banner.

the need for a banner-only option

Do we? We never had one and no one asked for it.

the appropriate name for that option is --banner with a required argument, defaulting to yes,

I'm just saying that the option doesn't turn on banner. Such an option should print the banner in this case.

yuyichao% julia --banner=yes -e 'println(1)'
1

Which is probably everyone else use a verb that corresponds to the surpression of message/warnings.

I also don't think that --quiet is a very good name for "suppress REPL warnings".

So as mentioned above it should be "quite startup". So just change it from -q, --quite to -q, --quite-startup should be enough. FWIW though, -q in gdb/python and -S in rr all have exactly the same behavior so I don't think it would be too surprising.

@StefanKarpinski
Copy link
Member Author

Do we? We never had one and no one asked for it.

I wanted it and asked for it by opening an issue. After guessing what the option to suppress the banner would be and trying --banner=no and then doing julia -h and finding that --quiet suppresses the banner and thinking, "huh, that's a bad name for an option to suppress the banner."

Such an option should print the banner in this case.
julia --banner=yes -e 'println(1)'

This is a good point and you could have just pointed that out instead of letting me guess at what you were talking about. I agree that should be changed, and as I said, it would be fine to re-introduce a --quiet option that is implicitly for quiet startup, including suppressing REPL warnings, and which implies --banner=no.

@yuyichao
Copy link
Contributor

yuyichao commented Aug 22, 2017

I wanted it and asked for it by opening an issue.

It seems that you just want to find the option, instead of wanting an option that only disable the banner but keep other initialization warning or messages.

trying --banner=no

Anything else that use this? For predictability I actually think -q is better. I was somewhat surprised that rr uses -S rather than -q. Also anything else that has a similar option without a short form? For an option that's only meaningful for the REPL, having a short form is pretty important.

could have just pointed that out instead of letting me guess

Well, I thought enable banner was clear enough and didn't meant to let you guess. So you also could have asked me for clarification instead of guessing.

@StefanKarpinski
Copy link
Member Author

I'm quite capable of deciding what I actually want for myself, thanks. The claim you made didn't seem to need clarification, it seemed flat out wrong – without the additional unguessable information that there is an interaction between --banner and -i. All of our options work like the new --banner option: the un-negated name of a thing you want to control and a value that option can take.

@StefanKarpinski StefanKarpinski added help wanted Indicates that a maintainer wants help on an issue or pull request good first issue Indicates a good issue for first-time contributors to Julia labels Aug 22, 2017
@StefanKarpinski
Copy link
Member Author

There are two actions to take here:

  1. Fix the interaction between --banner and interactive mode: julia --banner=yes -e0 should print a banner and exit. This requires a third value which means "yes if in REPL mode".

  2. Reintroduce a -q / --quiet option which implies --banner=no but also suppresses REPL warnings on startup.

@yuyichao, if you want to do either of those, great. Otherwise someone else can take this on.

cc @HarrisonGrodin, if you feel like doing some more option tweaking.

yuyichao added a commit that referenced this issue Aug 22, 2017
yuyichao added a commit that referenced this issue Aug 22, 2017
This reverts commit dcb46b3
and clarify the help message of `--quiet`.

Fix #23380
Reopen #23342
@JeffBezanson
Copy link
Member

Why would you ever want to print the banner when passing -e?

Also, maybe the "terminal not fully functional" warning is not useful and can just be removed.

@yuyichao
Copy link
Contributor

Why would you ever want to print the banner when passing -e?

I don't think you do since we already have Base.banner but it is what --banner=yes should mean. That's also why I'm not sure if we need --banner=* at all.

@JeffBezanson
Copy link
Member

--banner=yes doesn't have to mean that if we don't want it to.

@StefanKarpinski
Copy link
Member Author

If you ask for the banner, shouldn't you get it?

@JeffBezanson
Copy link
Member

In a way, the repl and the "script runner" are different programs, and --banner only really makes sense for the repl.

@yuyichao
Copy link
Contributor

and --banner only really makes sense for the repl.

I do think an error would be fine. Similarly for -q although the nice thing about quiet is that surpressing a non-existing banner/warning is less of an issue.

@StefanKarpinski
Copy link
Member Author

I've pushed an alternate fix to this issue that keeps --banner and allows banner printing to be controlled separately from --quiet which implies --banner=no but also suppresses REPL warnings.

StefanKarpinski added a commit that referenced this issue Aug 25, 2017
Make --quiet and --banner independent (fix #23380)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display and printing Aesthetics and correctness of printed representations of objects. good first issue Indicates a good issue for first-time contributors to Julia help wanted Indicates that a maintainer wants help on an issue or pull request REPL Julia's REPL (Read Eval Print Loop)
Projects
None yet
Development

No branches or pull requests

3 participants