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

#81 --quiet: prevent output of non-interactive commands #82

Merged
merged 1 commit into from
Apr 21, 2020

Conversation

possi
Copy link

@possi possi commented Jan 13, 2015

#81

@colinmollenhour
Copy link
Owner

Thanks for the PR, @possi. However, I am a little uncertain about merging this currently. Reasons being:

  1. Conventionally, the suppressable output (non-errors) should be output to stdout and the errors should be output to stderr. This way one can simply use > /dev/null to get rid of the non-error output.
  2. The feature is incomplete. If the --quiet option exists it should be implemented for all commands, not just repair and clean.

I think ideally the verbose output should be on stdout and warnings and errors on stderr. Then, the --quiet option would be used to suppress verbose output and warnings but not errors. So the options would be:

  • modman command (all output)
  • modman command > /dev/null (only warnings and errors)
  • modman command --quiet (only errors)
  • modman command > /dev/null 2>&1 (nothing)

@possi
Copy link
Author

possi commented Jan 15, 2015

I agree that the implementation is incomplete and could be optimized.

But I don't think that you should use stderr for warnings. In my opinion only errors (which causes the the return code to be != 0) should be send to stderr. It is usual, that you can use a --quiet option to suppress all the default output, e.g. if you like to use it in a cronjob.

  • modman command (all output)
  • modman command --quiet (only warnings and errors)
  • modman command >/dev/null (only errors)
  • modman command --quiet 2>/dev/null (only warnings, even if there is absolute no usecase for that)
  • and like always: modman command > /dev/null 2>&1 (nothing)

Explanation of my use case:
I'm having more than 10 modules, with each 5 or more links in it. Some of them 3rd-Party, some of them self-written. When I change any of the modules modman-file I need to repair. Executing "modman repair" gives more than one page of output (my terminal usual shows 48 lines).
That way I don't see if there's is something wrong, like typo in modman-file or such. Using "modman repair >/dev/null" in that case, gives the feeling of suppressing to much.

PS. An alternative would be, to not output all "Applied"-Lines if there is no -v|--verbose-Option.

@colinmollenhour
Copy link
Owner

Ok, I will concede that warnings could go to stdout if you prefer. I am against adding a verbose option to get the current behavior, though.

Is it really necessary to use "repair"? I can't even remember the last time I actually needed to use this. I think since the time it was added there have been improvements so that the only time it should be needed is if a line is completely removed from the modman file but the file it points to is not, and the symlink that is left over is able to break something by being there unintentionally.

@possi possi changed the title #81 modman repair --quiet #81 --quiet: prevent output of non-interactive commands Jul 14, 2015
@possi
Copy link
Author

possi commented Jul 14, 2015

Updated --quiet to affect more commands.

About using repair: For product I usually use deploy-all, but while creating new modules, or updateting them I prefer repair --quiet to check if every of our dozen submodules are configured correctly.

Also sometime it happens that we have to change directory-Symlink to directory/* what only can be done correctly using repair.

@Igloczek
Copy link

@colinmollenhour could you merge this or there are some other things that prevent adding this super useful feature?

@colinmollenhour colinmollenhour merged commit cd7c045 into colinmollenhour:master Apr 21, 2020
@colinmollenhour
Copy link
Owner

Merged.

@Igloczek
Copy link

Thanks! ❤️

@Igloczek
Copy link

@colinmollenhour sorry for interrupting you once again, but is there a chance to do a release with the merged code? There are quite few changes in the master branch since v1.12.

I know that the official installation way does not require it, but there are other ways, like homebrew, which require versioning, to work properly.

@colinmollenhour
Copy link
Owner

Sure, 1.13 tagged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants