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

cli: Add skipped packages to the transaction table #1440

Merged
merged 5 commits into from
May 10, 2024
Merged

Conversation

m-blaha
Copy link
Member

@m-blaha m-blaha commented Apr 23, 2024

Resolves: #359

m-blaha added 2 commits April 25, 2024 09:40
I'd like to enhance the functionality provided by this function to also
process skipped packages. Having the function as a method of
Transaction::Impl makes this easier.
get_broken_dependency_packages() - list of packages with broken
dependencies

get_conflicting_packages() - list of packages that have a conflict
@m-blaha m-blaha force-pushed the mblaha/skipped branch 2 times, most recently from 198baa4 to fdc76cf Compare April 25, 2024 08:15
@m-blaha m-blaha marked this pull request as ready for review April 26, 2024 10:40
@kontura kontura self-assigned this May 3, 2024
Copy link
Contributor

@kontura kontura left a comment

Choose a reason for hiding this comment

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

This looks good, I have only couple minor remarks.

Do you want to wait for #1191 or can we merge this before?

libdnf5-cli/output/transaction_table.cpp Outdated Show resolved Hide resolved
libdnf5-cli/output/transaction_table.cpp Outdated Show resolved Hide resolved
libdnf5-cli/output/transaction_table.cpp Outdated Show resolved Hide resolved
include/libdnf5/base/transaction.hpp Show resolved Hide resolved
libdnf5-cli/output/transaction_table.cpp Outdated Show resolved Hide resolved
libdnf5-cli/output/transaction_table.cpp Show resolved Hide resolved
m-blaha added 3 commits May 6, 2024 14:16
libsmartcols does not support spanning cells. Currently, the intertitles
in the transaction table (e.g., "Installing:", "Upgrading:",...) are
placed into the first column. Unfortunately, these titles for skipped
packages are quite long ("Skipping packages with conflicts:" and
"Skipping packages with broken dependencies:") and cause the first
column to become unnecessarily wide, breaking the transaction table.

To resolve this issue, this patch splits the transaction table into
several sections and keeps intertitles out of the table. Each section
of the table is then printed one at a time with respective titles using
the scols_table_print_range() function.

However, there are several issues with this approach:

- cannot utilize libsmartcols to print table headers. This is
  workarounded by adding the header manually as the first section of the
  table.
- cannot use the SCOLS_FL_TREE flag for tree-like indentation. This
  is workarounded by creating indentation manually using spaces.
@m-blaha
Copy link
Member Author

m-blaha commented May 10, 2024

This looks good, I have only couple minor remarks.

Do you want to wait for #1191 or can we merge this before?

Unfortunately it doesn't look that it will get merged in near future - with need to also clearly display the vendor changes it might not be possible to put everything on one line.

@kontura kontura added this pull request to the merge queue May 10, 2024
Merged via the queue into main with commit 388e253 May 10, 2024
13 of 19 checks passed
@kontura kontura deleted the mblaha/skipped branch May 10, 2024 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing section in transaction table: Skipping packages with broken dependencies
2 participants