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

systemctl: hide the 'glyph' column when --no-legend is requested #15081

Merged
merged 2 commits into from Mar 11, 2020

Conversation

mrc0mmand
Copy link
Member

@mrc0mmand mrc0mmand commented Mar 11, 2020

The former implementation of systemctl list-units and machinectl list hid the glyph column when called with --no-legend. This somehow got lost during the conversion to the format-table.[ch] stuff.

Fixes: #15077

As the formatting stuff doesn't allow hiding a specific column without giving the table_set_display() function the whole display map except the columns we want to hide, I wrote a simple function just for that, otherwise the code would get really ugly (in systemctl list-units we want to hide column 0 when --no-legend is specified and column 5 when the job list is empty). However, as the table formatting mechanism allows specifying columns to show/hide only when the display map is set, the function depends just on that, which feels... wrong, thus any comments are definitely welcome.

@poettering, @keszybz PTAL.

Copy link
Member

@keszybz keszybz left a comment

Choose a reason for hiding this comment

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

I think this is OK. It doesn't have to be the most efficient code. Clarity is more important.

But I think it'd help to do the move suggested below. Less chance of forgetting to do the initialization later on.

src/systemctl/systemctl.c Outdated Show resolved Hide resolved
src/systemctl/systemctl.c Outdated Show resolved Hide resolved
src/systemctl/systemctl.c Outdated Show resolved Hide resolved
src/shared/format-table.c Outdated Show resolved Hide resolved
@mrc0mmand mrc0mmand force-pushed the systemctl-hide-glyph-column branch 2 times, most recently from e17264d to c751794 Compare March 11, 2020 13:28
src/shared/format-table.c Outdated Show resolved Hide resolved
@keszybz keszybz added the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Mar 11, 2020
@mrc0mmand mrc0mmand removed the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Mar 11, 2020
@keszybz keszybz added the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Mar 11, 2020
@keszybz keszybz added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Mar 11, 2020
@anitazha anitazha merged commit 782a7eb into systemd:master Mar 11, 2020
@mrc0mmand mrc0mmand deleted the systemctl-hide-glyph-column branch March 11, 2020 20:44
dgdavid added a commit to yast/yast-storage-ng that referenced this pull request Oct 14, 2021
Needed to avoid getting the `glyph` column which is causing the issue
reported at https://bugzilla.suse.com/show_bug.cgi?id=1191347 about not
unmasking systemd mount and swap units when leaving the partitioner.

At some point --no-legend systemd flag was enough but it is
not the case anymore. See systemd/systemd#15081.
dgdavid added a commit to yast/yast-storage-ng that referenced this pull request Oct 14, 2021
Needed to avoid getting the `glyph` column which is causing the issue
reported at https://bugzilla.suse.com/show_bug.cgi?id=1191347 about not
unmasking systemd mount and swap units when leaving the partitioner.

At some point --no-legend systemd flag was enough but it is
not the case anymore. See systemd/systemd#15081.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed systemctl util-lib
Development

Successfully merging this pull request may close these issues.

New systemctl list-units table format breaks zsh completions
3 participants