Skip to content

Conversation

@bagder
Copy link
Member

@bagder bagder commented Jan 3, 2026

Time output:

  • was broken for really large times

  • now uses spaces instead of --:--:--

  • >99999y is now the largest shown time

  • show HH:MM:SS as long as hours are fewer than 100

Size:

  • made the decimal output always only use a single decimal

Test:

  • Add test 1622 to verify these functions

Time output:

- was broken for really large times

- now uses spaces instead of --:--:--

- >99999y is now the largest shown time

- show HH:MM:SS as long as hours are fewer than 100

Size:

- made the decimal output always only use a single decimal

Test:

- Add test 1622 to verify these functions
@bagder bagder requested a review from Copilot January 3, 2026 22:36
@bagder bagder marked this pull request as ready for review January 3, 2026 22:36
Copy link

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.

Pull request overview

This pull request fixes issues with large time output displays and improves decimal size formatting in the curl progress meter. The changes ensure that extremely large time values are properly handled and displayed, while size values now consistently use a single decimal place.

Key changes:

  • Modified time2str() to handle very large time values (up to >99999 years) with appropriate formatting transitions
  • Changed empty/zero time display from --:--:-- to spaces
  • Updated max5data() to consistently show only one decimal digit for size values
  • Added comprehensive test coverage through test1622

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/tool_progress.c Updated time2str() to support days, months, and years formatting for large values; modified max5data() decimal calculation for consistent single-digit precision
src/tool_progress.h Added UNITTEST declarations for max5data() and time2str() functions, and forward declaration for struct per_transfer
tests/tunit/tool1622.c New unit test file that validates time2str() and max5data() functions with various input values
tests/tunit/Makefile.inc Added tool1622.c to the test build
tests/data/test1622 Test data file with expected outputs for time and size formatting tests
tests/data/Makefile.am Added test1622 to the test data files list

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bagder
Copy link
Member Author

bagder commented Jan 3, 2026

augment review

@augmentcode
Copy link

augmentcode bot commented Jan 3, 2026

🤖 Augment PR Summary

Summary: This PR improves the curl tool progress meter’s formatting for both time durations and transferred-size values, especially at extreme ranges.

Changes:

  • Expose max5data() and time2str() to the unit-test build via UNITTEST and add prototypes under UNITTESTS.
  • Fix max5data() so the “decimal” size form consistently emits a single decimal digit (0–9) instead of variable-width decimals.
  • Rework time2str() to handle very large durations without breaking formatting, while keeping output fixed at 8 characters.
  • Change the “unknown/zero” time display from --:--:-- to 8 spaces to preserve column alignment.
  • Add additional time formatting tiers beyond 999 days (months, then years), capping display at >99999y.
  • Adjust progress meter local buffers to match the 8-char time string (+ NUL) requirement.
  • Add new unit test 1622 to validate time/size formatting over a wide range of values.

Technical Notes: Time formatting stays as HH:MM:SS while hours < 100, then switches to larger units to avoid overflow/width issues while maintaining a fixed-width progress layout.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@bagder bagder closed this in b32e66a Jan 3, 2026
@bagder bagder deleted the bagder/tool-progress-test branch January 3, 2026 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

1 participant