Skip to content

last: fix some hostnames are not shown properly with --hostname option issue 212#246

Merged
cakebaker merged 13 commits intouutils:mainfrom
LionelMeli:util-linux_last_issue_212
May 8, 2025
Merged

last: fix some hostnames are not shown properly with --hostname option issue 212#246
cakebaker merged 13 commits intouutils:mainfrom
LionelMeli:util-linux_last_issue_212

Conversation

@LionelMeli
Copy link
Copy Markdown
Contributor

No description provided.

@sylvestre
Copy link
Copy Markdown
Contributor

could you please add a test to make sure we don't regress? thanks

@sylvestre sylvestre requested a review from Copilot March 8, 2025 09:41
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR addresses an issue where the hostname is not displayed properly when using the --hostname option by refactoring the formatting logic.

  • Reformats the time and end time output by replacing a computed width with fixed-width fields.
  • Moves the hostname output to a separate conditional block to ensure it is printed correctly when required.

Reviewed Changes

File Description
src/uu/last/src/platform/unix.rs Adjusts formatting logic for time, end time, and hostname output.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

src/uu/last/src/platform/unix.rs:525

  • [nitpick] Consider replacing the magic number 12 with a named constant for clarity and maintainability.
write!(buf, " {time:<12}").unwrap_or_default();

src/uu/last/src/platform/unix.rs:526

  • [nitpick] Consider replacing the magic number 18 with a named constant for clarity and maintainability.
write!(buf, " {end_time_delta:<18}").unwrap_or_default();

@LionelMeli
Copy link
Copy Markdown
Contributor Author

LionelMeli commented Mar 11, 2025

Hello,

the last command doesn't work in mac OS and as I understand cannot work per this post:
https://unix.stackexchange.com/questions/468846/macos-what-file-does-the-command-last-get-its-information-from
and this one: https://superuser.com/questions/128705/ssh-last-login-last-and-os-x#:~:text=It%20seems%20modern%20Mac%20OS which explains " It seems modern Mac OS X records utmp/wtmp/lastlog-like information in the Apple SysLog (asl) database files at /var/log/asl/*."
In fact, after created a Mac OS VM (ventura thanks to quickemu project), the file /var/run/utmpx contains only the 2 entries (overwritten at each login). The uucore::utmpx::Utmpx doesn't fit the Mac OS requierement.

Maybe better to restrict the last command to the (true) *Nix platform? Let's me know.

@LionelMeli
Copy link
Copy Markdown
Contributor Author

Hello @sylvestre,
Could I do the same thing as per this PR: #234 (so mark things as unimplemented for systems other than Linux such as macOS)?

@LionelMeli
Copy link
Copy Markdown
Contributor Author

Hello,
@cakebaker @sylvestre, any opinions about to put the last command aside for macOS?
Thanks.

@cakebaker
Copy link
Copy Markdown
Contributor

@LionelMeli yes, doing it like in #234 is fine.

Comment on lines +130 to +134
let ouput = String::from_utf8(result.stdout().to_vec())
.unwrap_or("Failed to convert output to string".to_string());
let mut output_result: Vec<&str> = ouput.split('\n').collect();
// Remove the last 3 lines to compare easier with the expected output (so without the information about the begin of file)
output_result.truncate(output_result.len() - 3);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you can simplify this code to make the intent clearer with something like:

Suggested change
let ouput = String::from_utf8(result.stdout().to_vec())
.unwrap_or("Failed to convert output to string".to_string());
let mut output_result: Vec<&str> = ouput.split('\n').collect();
// Remove the last 3 lines to compare easier with the expected output (so without the information about the begin of file)
output_result.truncate(output_result.len() - 3);
let output_result: Vec<_> = result.stdout_str().lines().take(3).collect();

write!(buf, " {time:<time_size$}").unwrap_or_default();
write!(buf, " {end_time:<8}").unwrap_or_default();
if self.time_format != "notime" {
let time_ftm = 12;
Copy link
Copy Markdown
Contributor

@cakebaker cakebaker May 8, 2025

Choose a reason for hiding this comment

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

I think there is a typo and it should be fmt instead of ftm.

@cakebaker cakebaker merged commit 55c8ee0 into uutils:main May 8, 2025
13 checks passed
@codecov
Copy link
Copy Markdown

codecov bot commented May 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (1dcb552) to head (e1c094d).
Report is 6 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff     @@
##   main   #246   +/-   ##
===========================
===========================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cakebaker
Copy link
Copy Markdown
Contributor

Thanks for your PR!

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.

4 participants