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

Added de-duplication for environment list print #1503

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bc-lee
Copy link
Contributor

@bc-lee bc-lee commented May 18, 2024

Previously, dnf5 environment list would print duplicate environment with the same id and name. It would be confusing for the user to see duplicate environment names. This patch introduces de-duplication for the environment list print feature to merge duplicate environment entries. The original order of the environments is preserved by assigning an index value to each environment entry.

Fixes: #1502

Previously, `dnf5 environment list` would print duplicate environment
with the same id and name. It would be confusing for the user to see
duplicate environment names. This patch introduces de-duplication for
the environment list print feature to merge duplicate environment
entries. The original order of the environments is preserved by
assigning an index value to each environment entry.

Fixes: rpm-software-management#1502
@pkratoch
Copy link
Contributor

Hi, thanks for the patch! I agree it should be de-duplicated, but I'd rather have the code same as the one for the groups where it's in the dnf5/commands part: https://github.com/rpm-software-management/dnf5/blob/main/dnf5/commands/group/group_list.cpp#L73-L84 . It will be easier to maintain.

I also wanted to say that another advantage is that the de-duplication will work for both list and info, but unfortunately, the environment command doesn't reuse the list subcommand in the same way as group command does. But that's something we should change as well, though, it's not necessary to resolve in this PR, I can create a follow-up issue.

@bc-lee
Copy link
Contributor Author

bc-lee commented May 28, 2024

Thanks for the feedback. I appreciate it. I recognize that my solution in this PR isn't perfect, even from my own perspective. I'll take another look at it when I get a chance and see if I can improve it.

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.

Duplicated environment names in dnf5 environment list
2 participants