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

Use QCommandLineParser for command line args parsing #543

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

vifactor
Copy link
Collaborator

@vifactor vifactor commented Sep 23, 2024

As a side effect this PR removes some code duplication which had to be kept in sync + cmd parsing functionality is now handled by the Qt and the logic for combining those args is tested (at least those combinations that are given in examples)

@vifactor vifactor marked this pull request as ready for review September 23, 2024 20:43
@vifactor vifactor marked this pull request as draft September 27, 2024 19:51
- avoid code duplication by reusing help text in main window

- add tests
Add QDltOptManager::reset method to have possibility
to test the singleton

Signed-off-by: Viktor Kopp <[email protected]>
@vifactor vifactor marked this pull request as ready for review October 1, 2024 19:49
@vifactor
Copy link
Collaborator Author

vifactor commented Oct 7, 2024

@alexmucde you asked to add "ready"-label when PR is ready, since I do not have rights to add labels, I notify you with the message, that this PR is ready for review

@alexmucde alexmucde added this to the Release v2.27.0 milestone Oct 9, 2024
@alexmucde alexmucde self-requested a review October 9, 2024 11:22
@alexmucde alexmucde merged commit 5bab5b5 into COVESA:master Oct 9, 2024
4 checks passed
alexmucde added a commit that referenced this pull request Oct 10, 2024
@alexmucde
Copy link
Collaborator

@vifactor I had to reverse this change, as it caused several issues on my side and other users. E.g. the order of the parameters are now important. On my side export to CSV was not working anymore, perhaps also an issue of the order. Please provide a new PR when things are fixed.

@alexmucde
Copy link
Collaborator

@vifactor Please provide the change also for the tool dlt-commander, if possible.

@alexmucde
Copy link
Collaborator

@vifactor The export works, but the order was important for the -t option;
This worls: dlt-viewer input.dlt filter.dlf -t -c out.dlt -d -s, but -t at the end does not work

@vifactor
Copy link
Collaborator Author

@alexmucde first of all sorry for the troubles. Thanks for providing the exact failing command, I was about to ask what exactly fails. I'll try to investigate, should not be hard with the unit tests.

@alexmucde
Copy link
Collaborator

@vifactor I gave you also the "Write" role in the project. Please try if you can now set labels in the PRs and perhaps you can also push branches directly to the project.

@vifactor
Copy link
Collaborator Author

I tried: I can now assign labels. Thank you, I appreciate the trust!

@vifactor
Copy link
Collaborator Author

vifactor commented Oct 12, 2024

@alexmucde I can't reproduce the issue you mentioned. The commands dlt-viewer input.dlt filter.dlf -t -c out.dlt -d -s and dlt-viewer input.dlt filter.dlf -c out.dlt -d -s -t work exactly the same way on my machine. I use Ubuntu 22.04 with Qt 6.2.4

Could you please specify your OS, Qt version and how exactly the command misbehaves for you? If possible, also provide several exact commands that failed (for example I successfully converted a dlt to csv from command line using dlt-viewer input.dlt filter.dlf --csv -t -c txt.txt)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants