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

Allow the user to prefix our flags' names #54

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gurjeet
Copy link

@gurjeet gurjeet commented Feb 10, 2022

This problem has been reported quite a few times over the years; for
example, see it reported at golang-nuts mailing list 1, and cue lang
issue 2.

The problem is that, that glog package registers some flags in its
init() function. The list of registered flags also includes the -v
flag, which is usually used by developers either to control verbosity of
their code-execution, or to show the software version. It's notable that
all the complaints are regarding the -v flag, and none about the other
flags, since those other flags are unlikely be used by any other
developer.

The proposed fix allows the user of the glog package to change/prefix
glog's flags' names, so that they will not conflict with any flags that
they want to use.

This approach to the problem has a few advantages, compared to other
options like, disabling all the flags in glog.

  1. The default behaviour of the glog library is unchanged. So the
    current users of the library will not be affected.

  2. Any new users who wish to use the -v, or other glog-occupied flag,
    can do so at build time.

  3. The new users can still use the glog features/flags, albeit with a
    prefix.

  4. We are not enforcing some specific prefix, which may also conflict.

  5. The --help flag, correctly reports the changed/prefixed flag names.

$ ./main --help
Usage of ./main:

  ... other glog: prefixed flags ...

  -glog:v value
        log level for V logs
  -glog:vmodule value
        comma-separated list of pattern=N settings for file-filtered logging
  -v value
        Emit verbose execution progress

Please also see sample code 3 that demonstrates the problem, and how
the patch fixes the problem.

This problem has been reported quite a few times over the years; for
example, see it reported at golang-nuts mailing list [1], and cue lang
issue [2].

[1]: https://groups.google.com/g/golang-nuts/c/vj8ozVqemnQ
[2]: cue-lang/cue#1199

The problem is that, that glog package registers some flags in its
init() function. The list of registered flags also includes the `-v`
flag, which is usually used by developers either to control verbosity of
their code-execution, or to show the software version. It's notable that
all the complaints are regarding the `-v` flag, and none about the other
flags, since those other flags are unlikely be used by any other
developer.

The proposed fix allows the user of the glog package to change/prefix
glog's flags' names, so that they will not conflict with any flags that
they want to use.

This approach to the problem has a few advantages, compared to other
options like, disabling all the flags in glog.

1. The default behaviour of the glog library is unchanged. So the
current users of the library will not be affected.

2. Any new users who wish to use the -v, or other glog-occupied flag,
can do so at build time.

3. The new users can still use the glog features/flags, albeit with a
prefix.

4. We are not enforcing some specific prefix, which may also conflict.

5. The --help flag, correctly reports the changed/prefixed flag names.

```
$ ./main --help
Usage of ./main:

  ... other glog: prefixed flags ...

  -glog:v value
        log level for V logs
  -glog:vmodule value
        comma-separated list of pattern=N settings for file-filtered logging
  -v value
        Emit verbose execution progress

```

Please also see sample code [3] that demonstrates the problem, and how
the patch fixes the problem.

[3]: https://github.com/gurjeet/glog_fix_test
@robpike
Copy link
Contributor

robpike commented Feb 10, 2022

You are welcome to fork the code, but as it says in the README:

"The master copy of the source lives inside Google, not here. The code in this repo is for export only and is not itself under development. Feature requests will be ignored."

@gurjeet
Copy link
Author

gurjeet commented Feb 10, 2022

Thanks for pointing that out @robpike. I did read that warning, but I created this issue/PR since it's arguably a bugfix, and not a new feature request.

As suggested in the README, I have subscribed to and sent an email to the golang-nuts mailing list, but for some reason my email has not shown up in the public, yet. Perhaps my email is stuck in moderation queue.

In any case, I'll keep trying to engage the community on the mailing list, and keep this issue for posterity.

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.

2 participants