Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

eliminate misleading flag parsing error #1324

Merged
merged 1 commit into from
Aug 20, 2015

Conversation

rade
Copy link
Member

@rade rade commented Aug 18, 2015

The standard mflags error reporting produces an error like

flag provided but not defined: -p
See '/home/weave/weaver --help'.

on stderr. This is misleading since the program name is container local, so not something the user actually invoked. Furthermore, what the user can invoke doesn't have a --help option.

With this change the error becomes

FATA: 2015/08/18 10:56:23.635119 Incorrect usage: flag provided but not defined: -p

There are a few issues with this:

  • we no longer get a usage message
  • we no longer get deprecation warnings logged

Fixes #1321. Alas I am not at all happy with this. Wasted several hours on this already. Somebody else please have a go.

@awh awh self-assigned this Aug 20, 2015
@awh
Copy link
Contributor

awh commented Aug 20, 2015

vagrant@vagrant-ubuntu-vivid-64:~/weave$ ./weave launch -p
The weave container has died. Consult the logs with 'docker logs weave' for further details.
vagrant@vagrant-ubuntu-vivid-64:~/weave$ docker logs weave
flag provided but not defined: -p
See 'weave help'.

Feel free to squash if you're happy with the approach :trollface:

@awh awh assigned rade and unassigned awh Aug 20, 2015
@awh awh force-pushed the 1321_elminate_misleading_flag_parsing_error branch 2 times, most recently from 6f93a65 to 68bbc2d Compare August 20, 2015 13:51
@rade rade force-pushed the 1321_elminate_misleading_flag_parsing_error branch from 68bbc2d to c481d49 Compare August 20, 2015 14:50
@rade rade removed their assignment Aug 20, 2015
@rade rade changed the title WIP - eliminate misleading flag parsing error eliminate misleading flag parsing error Aug 20, 2015
@rade
Copy link
Member Author

rade commented Aug 20, 2015

@awh see what you make of this one. I've left the old commits in and reverted them, just in case we change our mind once more. If not, I'll squash them away.

@rade rade assigned awh Aug 20, 2015
@awh
Copy link
Contributor

awh commented Aug 20, 2015

That seems entirely reasonable to me, given the lack of flags support for overriding the executable name in sane fashion...

@awh awh assigned rade and unassigned awh Aug 20, 2015
@awh
Copy link
Contributor

awh commented Aug 20, 2015

The only other option appears to be vendoring mflags, which strikes me as wholly unappealing 😒

The standard mflags error reporting produces s.t. like
```
flag provided but not defined: -p
See '/home/weave/weaver --help'.
```
on stderr. This is misleading since the program name is container
local, so not something the user actually invoked.

Fixes #1321.
@rade rade force-pushed the 1321_elminate_misleading_flag_parsing_error branch from c481d49 to fd6d15e Compare August 20, 2015 16:21
rade added a commit that referenced this pull request Aug 20, 2015
…g_parsing_error

eliminate misleading flag parsing error

Fixes #1321.
@rade rade merged commit b075e93 into master Aug 20, 2015
@rade rade deleted the 1321_elminate_misleading_flag_parsing_error branch August 20, 2015 16:34
@rade rade modified the milestone: 1.1.0 Aug 29, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants