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

Change user info display on history command to include display name a… #1203

Merged

Conversation

derickdiaz
Copy link
Contributor

@derickdiaz derickdiaz commented Jan 25, 2024

Adds Display name and username to the history info command. This code assumes that the gecos information is in CSV format as normally this is standard. Based on that information it also assumes that the first entry within the gecos information is the display name.

Implements #358

This is an example of without and with the gecos information
Screenshot 2024-01-25 at 6 50 58 AM

Additionally if the user's information is not found, it will only display the user id as it has done originally.

@j-mracek j-mracek self-assigned this Jan 30, 2024
@@ -30,6 +30,7 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.
namespace libdnf5::cli::output {

void print_transaction_info(libdnf5::transaction::Transaction & transaction);
std::string generate_user_info_str(uint32_t user_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason why you want to add the function into public API? Be honest if there is no good reason I would prefer to move it in cpp to anonymous namespace and remove the declaration from HPP.


// If gecos information does exists, it is generally CSV formatted.
// This section assumes that the first entry in the gecos information is the display name.
if (user_info->pw_gecos != NULL && !std::string(user_info->pw_gecos).empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The condition if (user_info->pw_gecos != NULL && !std::string(user_info->pw_gecos).empty()) could be replaced by if (user_info->pw_gecos && strlen(user_info->pw_gecos) > 0) or if (user_info->pw_gecos != nullptr && strlen(user_info->pw_gecos) > 0).

It will not create any temporal object (std::string) to only check the length of char *

results += std::format(" {}", display_name);
};

if (user_info->pw_name != NULL && !std::string(user_info->pw_name).empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The condition if (user_info->pw_name != NULL && !std::string(user_info->pw_name).empty()) could be replaced by if (user_info->pw_name && strlen(user_info->pw_name) > 0) or if (user_info->pw_name != nullptr && strlen(user_info->pw_name) > 0).

It will not create any temporal object (std::string) to only check the length of char *

@derickdiaz derickdiaz force-pushed the history-info-user-output branch from 4685d9f to 4f135f1 Compare January 31, 2024 08:33
@j-mracek
Copy link
Contributor

LGTM

@j-mracek j-mracek added this pull request to the merge queue Jan 31, 2024
Merged via the queue into rpm-software-management:main with commit 2427a33 Jan 31, 2024
5 of 9 checks passed
@Conan-Kudo
Copy link
Member

This broke Mageia builds by silently raising the minimum GCC version to 13, see #1269 for details.

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.

3 participants