Repository navigation
flatpak-coredumpctl: Add type hints and refactor code to prepare for future PRs - #6677
Conversation
|
Would have been nice to make the new subcommand a separate PR after the refactorings were done. |
|
the flatpak-coredumpctl: Add 'flatpak-coredumpctl list' command commit is touching a lot of things such as reorganising argparse that is making hard to review, |
52bb50e to
55d8451
Compare
I can split it up into two if you prefer. I was originally going to do that, but then felt like the earlier refactoring didn't have much justification on its own when not paired with the new features. |
1966b39 to
0d1fdfd
Compare
I can try and split it up some. It definitely felt like a very large commit to me while I was doing it, but I was having a hard time of thinking of how to reasonably make smaller commits. |
|
Yes feel free to split this. |
-m with no argument currently results in coredumpctl_matches having a value of "", which is exactly the same as if it weren't used at all. As far as I can tell, allowing this does nothing useful, only making the usage slightly less clear.
There are a couple related changes made here, with the intention of making it easier to implement additional features in the future: - Mutual exclusivity of --build-directory and app is better enforced both through types and at runtime. The types will let type checkers ensure that we cannot reach the run function without at least one of them being valid. At runtime, it no longer allows both to be specified, which could result in confusion over which takes precedence. - Instead of wrapping the entire program in a class, there is now only a small class that handles validation of the arguments and holding the data to be passed to run. This provides a better separation of concerns, with the argument parsing now being a little less tied to running the debugger.
Help messages were inconsistent about whether they ended with periods or not. I chose to remove the periods from those that had them rather than the other way around because the help messages automatically generated by argparse do not have periods.
This just makes the generated messages slightly less redundant and a little closer to coredumpctl's.
0d1fdfd to
e71cbb8
Compare
|
Alright, I've made this PR contain only the refactoring and other minor changes. The list subcommand will be in a new PR. |
electricbrass
changed the title
flatpak-coredumpctl list subcommand
3 participants
This PR refactors
flatpak-coredumpctlto have clearer control flow and slightly more consistent help messages, with type hints added throughout for better static analysis. This is all largely to make future work on it easier and less error-prone.