ui: print to STDERR (play nice with UNIX pipes) #156

Merged
merged 1 commit into from Oct 10, 2011

Projects

None yet

6 participants

@sunaku
Contributor
sunaku commented Oct 9, 2011

Messages directed at the user should be printed to STDERR because when a guard program's STDOUT is connected to other programs via UNIX pipes, user messages interfere with valid guard output. Play nice!

@rymai
Member
rymai commented Oct 9, 2011

Hi, thanks for this, however I'm not sure I understand the difference between "user messages" and "valid guard output"?

The thing is you print all Guard's output to STDERR, shouldn't there be only the "error" messages printed to STDERR?

Maybe you can describe a bit more with examples what was the issue and how this pull-request resolves it? :)

Thanks.

@sunaku
Contributor
sunaku commented Oct 9, 2011

I use Guard's directory watching abilities in this simple script, whose STDOUT is consumed by a different program further downstream on a UNIX pipeline. The problem is that Guard is emitting the following error/warning messages to STDOUT instead of to the more suitable STDERR:

Please install rb-inotify gem for Linux inotify support
Using polling (Please help us to support your system better than that).

This interferes with the downstream processing of my simple wrapper script's actual output.

@sunaku
Contributor
sunaku commented Oct 9, 2011

You might ask: "why don't you install the suggested rb-inotify gem to avoid getting that Guard warning?" The reason is that such a task can only be performed by the end user who uses my scripts; I have no control over their machines. And when Guard's warning is printed to STDOUT, it is silently consumed by my downstream script and the end user never sees it. In contrast, if Guard printed warnings to STDERR, the end user would see them!

@rymai
Member
rymai commented Oct 9, 2011

Ok I see the point, but Guard doesn't print only warnings (for example here, what do you think we should we do with them?

@sunaku
Contributor
sunaku commented Oct 9, 2011

I would print those to STDERR also. They are meant for the user, not programs consuming Guard's actual output.

Think of STDERR as a global message bus (such as growl notifications) that is simultaneously available to all programs in a UNIX pipeline. You can use them to notify the user instantly from anywhere in the pipeline. Also, because it is global, you should prefix your output with the program name:

STDERR.puts "guard: some message here"
# or, an even shorter way to do it:
warn "guard: some message here"
@netzpirat
Contributor

My first reaction on this was that only errors should go to STDERR, as the name implies. But after I read the following description of STDERR:

Standard error is another output stream typically used by programs to output
error messages or diagnostics.

at Wikipedia, I think that at least error, deprecation and debug should definitely write to STDERR. Writing info messages to STDERR feels wrong, but when you think about Guard just being a framework for Guard implementations, then everything from within Guard should be considered as diagnostics.

We are free to make a general decision here without impact, because all Guards I have seen so far are doing something with the files and the output are again files or processes and all messages could be considered as diagnostics. This is the first use where the output written to STDOUT is used as output to be further processed. From a user perspective nothing changes, it works and looks the same as before.

I vote +1 for this, but in case we merge it, a description should go into the documentation of the ui.rb class, that everything logged through this class is considered an error or diagnostic messages, and writing to STDOUT should be done by simply using put.

@sunaku
Contributor
sunaku commented Oct 9, 2011

Well said. Going further, since the purpose of UI is to convey Guard diagnostics, perhaps a Logger connected to STDERR could replace UI entirely:

UI = Logger.new(STDERR)
@netzpirat
Contributor

Logger is 1.9 only and I use the private color method in Guard::UI from within my own Guards :P I'm tending more to make them public.

@sunaku
Contributor
sunaku commented Oct 9, 2011

Does a color implementation really belong in Guard? I would use a different library for that.

@netzpirat
Contributor

Yes, it would make sense to use term-ansicolor, but I don't like projects where pull requests and issues are completely ignored.

@sunaku
Contributor
sunaku commented Oct 9, 2011

True, that can be disheartening. I've dealt with my fair share of such cases and, in the end, learned to cope by just using my own fork as the "official" repository for my own projects. In this case, @flori seems to be active on his other projects, so there's a good chance he would be willing to look at term-ansicolor's pending issues.

@netzpirat
Contributor

Yes, perhaps everything will be fine with term-ansicolor. I've no problem with changing that at all, I'd have changed my Guards within a few minutes. But there are more important things to work on Guard than replacing a few lines of perfectly working code. But if you like to take care about it (including answer issues from crying Windows users) and @thibaudgg has no historical reason against it, then feel free to send another pull request.

@thibaudgg
Member

Yeah I totally agree on what @netzpirat, but if you want to change that I see no problem about it. Maybe we can use https://github.com/defunkt/colored, feel free to send another pull request.
In the meantime, I merge this one. Thanks!

@thibaudgg thibaudgg merged commit 6f1a0ac into guard:master Oct 10, 2011
@flori
flori commented Oct 10, 2011

It's possible to switch of coloring globally by calling
Term::ANSIColor::coloring = STDOUT.isatty
at the beginning of your script for exactly this use case, piping to other programs.

@rymai
Member
rymai commented Oct 10, 2011

Hey guys,

sorry I'm a bit late but I don't feel comfortable with writing what's not diagnostic to me to STDERR. Take for example the RSpec guard: it runs specs and display the summary. I agree that the main process here is the specs that are run, BUT if I'm not there and can't see a visual (or sound, or whatever signal that my senses can feel) output, it's worthless (unless I have a script that use the return value and run something else or whatever), so I think in this case it's not diagnostic.

The problem here is simply that messages outputted by Guard itself (for instance by Guard::Listener(::Darwin)) should be only warning/error/deprecation/debug and should be outputted to STDERR. BUT guards implementation should be able to choose whether they want to output warning/error/deprecation/debug messages (=> STDERR) or normal/info/actually-useful messages (=>STDOUT). The thing with the new implementation is that guards who want to output to STDOUT will use puts (this is fine) but in doing so they now bypass any Guard's control/decoration/whatever. We're losing control guys!!!!!! :P

We should definitely keep UI.info => STDOUT in order to keep guards under control! :D

lol, do you get what I'm trying to say? ^^

Regarding the color issue, I agree to externalize it.

@netzpirat
Contributor

You see messages written to STDERR in your console, so from a user perspective nothing has changed:

$ echo "TEST" > /dev/stderr 
TEST

All those message you see from RSpec are diagnostics messages, because it has no real output that you process further. You don't lose control, because you never had control in a dynamic language like Ruby and every developer was always free to use puts.

@netzpirat
Contributor

Since you cannot have control, you can only lay out some guidelines and hope developers read and follow them. So we have definitely made a step in the right direction by just defining what to use when.

@rymai
Member
rymai commented Oct 10, 2011

I know that STDERR is by default written to my console, that's not my point here, it's not because users will not see anything changing that it's the right decision.

Regarding the RSpec example, I don't agree because if it's diagnostics it means that I can skip these messages entirely. But I don't because my development rely on this, even if no other program is using this output.

Regarding the "losing control" joke (in the first place), what I meant is that currently guards are advised to use the Guard::UI class to output messages to the console, in that sense we have some control (for example if ENV['GUARD_ENV'] is true, we don't output anything), and yes I know I'm free with Ruby, come on Michael! ;)

@netzpirat
Contributor

My comment wasn't meant to teach you, it was just an argument against losing control. I find it very hard to come to a clear decision, I see arguments on both sides. Not speaking personal and not even in my mother tongue makes it even harder. I think you best discuss it with @thibaudgg face to face and make a decision. I'm fine with everything you'll decide.

@thibaudgg
Member

Ok, I'll discuss that face to face with @rymai tomorrow :) Thanks for your feedback @netzpirat

@rymai
Member
rymai commented Oct 10, 2011

No problem Michi! No big deal, really!

It's just that it seems to me that @sunaku has proposed a solution that fixes his particular problem, that's fine but there were other solutions to it, that could maybe be better. We'll discuss that tomorrow with the boss. :P

@kierzniak

In my project I'm using guard to some standard stuff SASS ,Livereload etc. I'm trying to capture output to change it a little bit to fit my project. Because of using STDERR insted of $stderr I'm unable to capture output from Guard.start method using popular method:

def capture_stderr
  previous_stderr = $stderr
  $stderr = StringIO.new
  yield
  $stderr.string
ensure
  $stderr = previous_stderr
end

I can't change $stderr to STDERR in this method because STDERR is a constant. I would change STDERR to $stderr everywhere in the Guard. This is quick fix.

In more elegant way would be nice to have module with all Guard messages and overwrite it in my own project. What do you think?

@thibaudgg
Member

I always in favor of the elegant way, @guard/core-team what do you think?

@netzpirat
Contributor

Sure, why not... but it would only affect Guard messages and not the ones from each Guard plugin.

@kierzniak

You have right. But I think this is rather solution for libraries which is using guard and will be implementing own solutions. However in future major version guard can force plugins to implement some kind of interface or abstract class to messages.

If you know better solution to customize terminal or notification output please let me know. Unfortunately method capture_stderr which i mention before can only hide output. I don't have capabilities to redirect and change it.

@netzpirat
Contributor

Guard is not really meant to be used as a library, it's rather a customizable product to improve the developers workflow. Sure, you can use it programmatically and write plugins for it, but it's always limited to some extend to "the Guard way". If you want to use it in another way, you can make use of Listen and write your own workflow and interactions around it.

Forcing the existing plugins to adhere to some kind of messaging is not a good idea, because there are many plugins that are working fine with the current API, but aren't really maintained and thus we will split half of the plugin ecosystem. Deprecation messages are not effective, because the authors of the unmaintained plugins won't see them and the active Guard users will suffer under the message flood. Anyway, I've seen Guard plugins that aren't using the provided Guard::UI class anyway and just write to stdout, so we cannot enforce it anyway.

You can always open Guard in a subprocess and filter the stream, so you can control the console output and you can write your own notifications in the parent process depending on it. If you disable the notifications in the subprocess, you have full control of the Guard output.

@kierzniak

You have right, probably listen will be better for my project.

@dichen001 dichen001 referenced this pull request in dichen001/Paper-Reading Jun 28, 2016
Open

Summary of the 20 issues in Herbsleb's 2014 FSE paper. #6

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