Skip to content

printf: Add indexing to format strings #7837

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

Merged
merged 1 commit into from
Apr 25, 2025

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

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 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