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

Add :test:function, :test:select, and :test:format function definitions #769

Merged
merged 9 commits into from
May 13, 2024

Conversation

eemeli
Copy link
Collaborator

@eemeli eemeli commented Apr 15, 2024

CC @mradbourne

Given that we shouldn't rely on :number and :datetime producing exact results but we still want to ensure that our interfaces with them work, let's add :test:number and :test:plural as well-defined functions intended for tests only, with exactly defined behaviour.

This PR could be extended with test cases making use of these, but I figured it'd be more useful to consider the general case first, esp. in order to avoid conflicting with #767.

test/README.md Outdated Show resolved Hide resolved
@eemeli eemeli changed the title Add test:number and test:plural function definitions Add test:func, test:select, and test:format function definitions Apr 16, 2024
@eemeli
Copy link
Collaborator Author

eemeli commented Apr 16, 2024

As discussed during yesterday's call, it'd be better for the functions to more clearly depart from e.g. :number.

I also added details for what happens when a test:func value is used as an argument of another test:func expression.

@eemeli eemeli requested a review from aphillips April 16, 2024 08:49
test/README.md Outdated Show resolved Hide resolved
test/README.md Outdated Show resolved Hide resolved
test/README.md Outdated Show resolved Hide resolved
test/README.md Outdated Show resolved Hide resolved
@eemeli eemeli changed the title Add test:func, test:select, and test:format function definitions Add :test:function, :test:select, and :test:format function definitions Apr 17, 2024
@eemeli eemeli requested a review from aphillips April 17, 2024 08:13
@mradbourne mradbourne mentioned this pull request Apr 19, 2024
test/README.md Outdated Show resolved Hide resolved
test/README.md Outdated Show resolved Hide resolved
test/README.md Outdated Show resolved Hide resolved
test/README.md Outdated Show resolved Hide resolved
@eemeli eemeli requested a review from aphillips May 6, 2024 07:20
Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

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

The proposed :test:function appears super minimal, which seems good to me. I agree this is useful for testing, giving a way to avoid locale-dependent output. I'll defer on the exact spec text and scope.

test/README.md Show resolved Hide resolved
test/README.md Outdated Show resolved Hide resolved
test/README.md Show resolved Hide resolved
test/README.md Show resolved Hide resolved
test/README.md Show resolved Hide resolved
test/README.md Show resolved Hide resolved
test/README.md Outdated Show resolved Hide resolved
#### Options

The only _option_ `:test:function` recognizes is `fd`,
a _digit size option_ for which only `0` and `1` are valid values.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Linkifying digit size just as "Number Operand" is being linkified 5 lines above. Helps short circuit questions like mine for the reader.

Suggested change
a _digit size option_ for which only `0` and `1` are valid values.
a [_digit size option_](/spec/registry.md#digit-size-options) for which only `0` and `1` are valid values.

Copy link
Member

Choose a reason for hiding this comment

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

Underscore italics are linkified when the spec is turned into HTML. That's what this is for.

@eemeli eemeli requested a review from echeran May 8, 2024 07:19
Copy link
Collaborator

@echeran echeran left a comment

Choose a reason for hiding this comment

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

LGTM

@aphillips aphillips merged commit 6414b6c into main May 13, 2024
2 checks passed
@aphillips aphillips deleted the test-func branch May 13, 2024 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants