Skip to content

some CmdLineParser-related cleanups#7105

Merged
firewave merged 1 commit intocppcheck-opensource:mainfrom
firewave:cmd-cleanup
Feb 5, 2025
Merged

some CmdLineParser-related cleanups#7105
firewave merged 1 commit intocppcheck-opensource:mainfrom
firewave:cmd-cleanup

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

No description provided.

@firewave
Copy link
Copy Markdown
Collaborator Author

I filed https://trac.cppcheck.net/ticket/13431 about consolidating the XML CLI options.

@firewave firewave force-pushed the cmd-cleanup branch 2 times, most recently from 9a0ad07 to 2608caf Compare January 3, 2025 10:03
@firewave firewave marked this pull request as ready for review January 3, 2025 10:03
@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Jan 3, 2025

Comment thread cli/cmdlineparser.cpp
Comment thread lib/settings.h
* Check code in the headers, this is on by default but can
* be turned off to save CPU */
bool checkHeaders = true;
bool checkHeaders = true; // TODO: CLI
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I did have a intention here. My idea was to only provide certain advanced options in the GUI. In an effort to keep the command line options simple.
It's a good thing if options are available on the command line.
It's a bad thing that the command line is "bloated".

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I did have a intention here. My idea was to only provide certain advanced options in the GUI. In an effort to keep the command line options simple.

Since this is a GUI project option it would be good to have an CLI option so you could override it since the project file might be immutable. Although CLI and GUI projects do not play well with each other at the moment - see https://trac.cppcheck.net/ticket/12918.

It's a good thing if options are available on the command line.
It's a bad thing that the command line is "bloated".

Well - that's kind of the point of a CLI - it should allow you to automate things all things you could do in the GUI. It also makes writing tests easier since you do not require to generate a project file.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can set the option in the gui project file though so you CAN use the cli and automate all things. It's a XML format file so not hard to write.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something like ffmpeg is an example of where I don't want to end up:
https://gist.github.com/tayvano/6e2d456a9897f55025e25035478a3a50

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can set the option in the gui project file though so you CAN use the cli and automate all things. It's a XML format file so not hard to write.

The GUI project is inferior to the CLI. It is not documented. It only has a subset of the CLI options. The handling is different or just missing because that logic lives in the CmdLineParser or the GUI description.

For this to work properly it would require a rework. The easiest way would be to treat it more like a front-end than a GUI by generating CLI options which are then fed to the CmdLineParser. This would actually simplify a lot of things and probably get rid of a lot of duplicated (and possibly untested) code.

Copy link
Copy Markdown
Collaborator

@danmar danmar Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but we also have some which we probably do not need to have

do you have some in mind? spontanously I am personally thinking about the --rule.

I would like to get rid of extra flags. What is a good approach for that. I am thinking that we could at least hide such options so they are not shown in --help output anymore.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could easily come up with 100 options that could benefit some users. If we add those to cppcheck it will become a user-unfriendly tool.

Could we provide "basic flags" and "advanced flags" somehow. If "basic flags" are fully visible in the normal help output but the "advanced flags" are not in the normal help but somehow the user can see those..

Do you know if some tool has such "advanced flags" how does it expose those?

How about only "basic flags" is written if you write "--help". And the --help output could say that "advanced options can be seen if you use --help-all" or something like that?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know if some tool has such "advanced flags" how does it expose those?

Yes. Several do that. They usually have a note in the output that you get more information if you pass a different flag. So --help-all would make sense.

It would make sense if we try to rework that with also moving the actual documentation of the options out of the source file so we can generate the manpage from it as well. There is a ticket about it but I did not find it right now.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we merge this? This does not introduce any new CLI options yet and there are several follow-ups being held up.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this to work properly it would require a rework. The easiest way would be to treat it more like a front-end than a GUI by generating CLI options which are then fed to the CmdLineParser. This would actually simplify a lot of things and probably get rid of a lot of duplicated (and possibly untested) code.

I filed https://trac.cppcheck.net/ticket/13623 for tracking this.

How about only "basic flags" is written if you write "--help". And the --help output could say that "advanced options can be seen if you use --help-all" or something like that?

I filed https://trac.cppcheck.net/ticket/13624 for tracking this.

Copy link
Copy Markdown
Collaborator

@danmar danmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry it took so long for the approval.

@firewave firewave merged commit fcda2d1 into cppcheck-opensource:main Feb 5, 2025
@firewave firewave deleted the cmd-cleanup branch February 5, 2025 17:47
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