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

Reapply "Use QCommandLineParser instead of custom cmd args parsing (#543)" #566

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

vifactor
Copy link
Collaborator

@vifactor vifactor commented Oct 14, 2024

PR employs QCommandLineParser for parsing cmd args for dlt-viewer. As requested, in a followup PRs similar change can be applied to other cli tools developed in this repo.

The PR restores the previously reverted changes, that reported to cause some issues. Should remain unmerged until fix is found (but only after clear issue is reported) or we can merge it again and if issue is observed, request to create a bug ticket with clear description.

@alexmucde I would need your help to understand what kind of issues this causes and how to reproduce it. Unit tests can help to ensure workability of different cmd scenarious.

@alexmucde
Copy link
Collaborator

@vifactor I will test the version

@alexmucde alexmucde added this to the Release v2.27.0 milestone Oct 17, 2024
@alexmucde
Copy link
Collaborator

I have tried a first use case, which did not work:
-c C:\traces\out.txt C:\traces\in.dlt -t
The conversion to ASCII was not working.

@vifactor
Copy link
Collaborator Author

from the path I conclude that it is on Windows. Thanks @alexmucde ,I'll investigate from here

@vifactor
Copy link
Collaborator Author

vifactor commented Oct 18, 2024

I do not know what I do differently, but for me it just works on Windows too:
image

@alexmucde
Copy link
Collaborator

@vifactor I will try again.

@alexmucde
Copy link
Collaborator

No it works also on my side. Do not know what i made wrong last time. I will test now further use cases.

@alexmucde
Copy link
Collaborator

The use cases i have tested work now. Should we give it another try now and merge it?

@vifactor
Copy link
Collaborator Author

I would give it a try, yes. Quite confident thanks of unit tests, but of course platform-specific bugs are not excluded, so if there is an issue would be nice to have a nice bug report

@alexmucde alexmucde merged commit 1e41b0d into master Oct 24, 2024
4 checks passed
@alexmucde
Copy link
Collaborator

@vifactor Tested it now also in my productive system in Ubuntu 24.04 and works fine.

@hannesa2 hannesa2 deleted the qcommandlineparser-fix branch October 24, 2024 09:03
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