Skip to content

Conversation

@sargas
Copy link
Contributor

@sargas sargas commented Apr 25, 2025

This PR implements the ability for printf (and the general formatting functionality in uucore) to have "conversion specifications" that explicitly say which argument to use. This comes from the Single Unix Specification and is tested for in GNU's tests/printf/printf-indexed test.

To handle arbitrary positions, I've changed the iterator-based approach to a slice of FormatArgument's, and wrapped that in a struct to keep track of the "next" argument to grab (as well as how the positions should be offset when repeatedly processing the format string

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/printf/printf-indexed is no longer failing!

@sylvestre
Copy link
Contributor

Congrats! The gnu test tests/printf/printf-indexed is no longer failing!

kudos!

@sylvestre sylvestre requested a review from Copilot April 25, 2025 21:52
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 PR implements support for positional conversion specifications in printf formatting by changing the argument handling from an iterator to a new FormatArguments struct, along with corresponding changes in parsing and format‐writing routines.

  • Added positional specifier support in format parsing (e.g. "%1$d", "%2$s", etc.).
  • Refactored argument handling using a stateful FormatArguments struct to support both sequential and positional argument access.
  • Updated and added tests to cover various scenarios with positional and mixed access patterns.

Reviewed Changes

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

Show a summary per file
File Description
tests/by-util/test_printf.rs Updated tests and added new test for positional format specifiers
src/uucore/src/lib/features/format/spec.rs Modified Spec enum and parsing functions to record argument positions
src/uucore/src/lib/features/format/num_format.rs Adjusted pattern matching for CanAsterisk to account for new positional syntax
src/uucore/src/lib/features/format/mod.rs Changed argument handling by using FormatArguments instead of iterators
src/uucore/src/lib/features/format/argument.rs Introduced and refactored FormatArguments to support positional indexing and batching
src/uu/printf/src/printf.rs Refactored printf to work with the new FormatArguments and batching logic

@sylvestre sylvestre merged commit 606c0c1 into uutils:main Apr 25, 2025
70 checks passed
@sargas sargas deleted the add-indexing-to-printf branch April 26, 2025 02:12
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.

2 participants