Repository navigation
flatpak-coredumpctl: Add flatpak-coredumpctl list subcommand - #6705
Conversation
swick
left a comment
swick
left a comment
There was a problem hiding this comment.
Seems fine, but a few commits should probably be merged (the ones which fix up the previously newly added feature).
4cbe906 to
942e7c8
Compare
|
Last two comments should get rid of the remaining typecheck errors. You can squash the first one with |
ee75bb4 to
2047f6f
Compare
|
I feel like the first commit and commits 3 to 6 should be squashed into one as it is introducing a new subcommand and new functionality entirely. The next ones are just improving on that new functionality which could be done from the get go. It causes an oddity that in 6c847d7 the Otherwise seems fine to me now. |
|
Sure, can do. I did it that way just because you'd previously said that adding the new stuff to argparse at the same time made it difficult to review. |
|
I probably wasn't clear enough. I think squashing those will better as there will be at least one commit in the history where |
Reworks the interface of flatpak-coredumpctl to use subcommands similar to those of coredumpctl. The new list subcommand is currently non-functional, but will list all store coredumps from flatpak applications, and the previous debugger functionality has been moved to the debug subcommand. flatpak-coredumpctl: Implement basic functionality for 'list' subcommand Resolves flatpak#2002: 'flatpak-coredumpctl list' now lists coredumps in a format similar to coredumpctl, except that it skips inaccessible coredumps by default flatpak-coredumpctl: Use a pager for list output These less flags don't perfectly match coredumpctl because it uses systemd's pager instead of less, but it's close enough flatpak-coredumpctl: Match coredumpctl's text styling The column titles are now underlined and missing/inaccessible coredumps are grayed out. This is probably just a little overengineered for what is needed, but I wanted to make it easy to expand in the future if needed, and it is still fairly minimal. flatpak-coredumpctl: Add matches argument to list subcommand
coredumpctl is required for both subcommands, and likely will be for any others added in the future, so we should just check for it in one place at the start.
This constraints the scope of the variables within instead of unnecessarily creating global variables, and it makes it easier to keep only a single sys.exit
Fixes a type check error with mypy Other members had their type hints moved to the class body as well for consistency
2047f6f to
9bf18df
Compare
3 participants
This is an updated version of the feature that was removed from #6677, and is stacked on the current version of that PR.
flatpak-coredumpctl listlists all coredumps from flatpaks, filtering coredumpctl's list using the same check as the existing debug functionality (does the path start with /app or /newroot). The format matches coredumpctl's as closely as possible, with one exception. Inaccessible and missing coredumps are hidden unless the--show-inaccessibleoption is used. If this is not wanted, I can remove it, but personally I find that I almost never want to see those and that it's just extra noise in coredumpctl's output.The existing debug functionality has been moved to the
debugsubcommand. This aligns with the interface of coredumpctl.Fixes: #2002