Skip to content

Can’t avoid line numbers with command line tool #479

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

Closed
kurtschelfthout opened this issue Oct 14, 2018 · 10 comments
Closed

Can’t avoid line numbers with command line tool #479

kurtschelfthout opened this issue Oct 14, 2018 · 10 comments

Comments

@kurtschelfthout
Copy link
Contributor

The command line tool has —lineNumbers which defaults to true. Unfortunately that means you effectively can not turn it off looks like - because CommandLineParser interprets absence of the arg as the default, and presence of the arg as true also.

Lmk if I missed anything here.

If not, any preference for changing the default to false, OR changing the switch to —noLineNumbers? Happy to PR either.

@wallymathieu
Copy link
Member

Is this flag used today by any project? It could be a remnant from earlier times. Adding ,-noLineNumbers flag should be safe, so that the CLI API is backwards compatible. Though, since the current version is in beta, there could be a reason to make a breaking change.

@kurtschelfthout
Copy link
Contributor Author

Is this flag used today by any project?

Who knows? :) If it's used, it's basically pointless. The Fake 5 wrapper for this tool does not expose any option to remove line numbers, so at least from that side there isn't any usage.

Your call.

@wallymathieu
Copy link
Member

Looks like @fbmnds and @Krzysztof-Cieslak are the ones who has touched the code lately (2-5 years), so could perhaps give some insights into the intent. I'm not in charge of this project. I try to help out because I like the library. I think @matthid is the one who does most of the work right now.

@kurtschelfthout
Copy link
Contributor Author

Just found #402...sorry for the dup.

@wallymathieu
Copy link
Member

I think the comment about a PR still stands. If you send in a PR that fixes the issue people would be happy to accept it.

@wallymathieu
Copy link
Member

There seems to be some documentation that mentions some of the behavior. Since the default is to have line numbers, perhaps better to change the semantics to be able to disable them. What does @matthid think?

@kurtschelfthout
Copy link
Contributor Author

Totally happy to PR, just want any maintainer to make a call on whether they'd prefer to switch the default of --lineNumbers to false (which would turn off linenumbers for everyone silently) or whether they'd prefer to remove --lineNumbers altogether and add a --noLineNumbers option instead (which keeps the current default but any current superfluous usage of --lineNumbers would fail. )

I would suggest to do the second, at least it doesn't silently change behavior.

@wallymathieu
Copy link
Member

I'd prefer the second alternative to be honest (since the current cli flag does not add value).

@matthid
Copy link
Member

matthid commented Oct 14, 2018

Yes I agree --noLineNumbers is fine as most people probably want line numbers and it is better to not break stuff if possible?

@wallymathieu
Copy link
Member

If you are paranoid about the usage, then you could have --lineNumbers as a dummy parameter that gives you warning. But I'm not sure if it's worth the effort.

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

No branches or pull requests

3 participants