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

Editorial: Fix spec link for and remove unused parameter from new ValidateDurationUnitStyle AO #969

Closed
wants to merge 2 commits into from

Conversation

trflynn89
Copy link
Contributor

@trflynn89 trflynn89 commented Mar 3, 2025

No description provided.

@trflynn89 trflynn89 changed the title Editorial: Fix spec link for new ValidateDurationUnitStyle AO Editorial: Fix spec link for and remove unused parameter from new ValidateDurationUnitStyle AO Mar 3, 2025
@@ -648,10 +648,9 @@ <h1>
}.
</emu-alg>

<emu-clause id="sec-stringpaddingbuiltinsimpl" type="abstract operation">
<emu-clause id="sec-validatedurationunitstyle" type="abstract operation">
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @trflynn89; I've merged this fix as #970.

<h1>
ValidateDurationUnitStyle (
_unit_: a String,
Copy link
Contributor

Choose a reason for hiding this comment

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

Inclusion of a seemingly-irrelevant unit parameter was intentional; I expect its contents to appear in the message of any thrown exception just as it would have before being factored out of GetDurationUnitOptions in 20daf26. But this is obviously too subtle... perhaps, rather than dropping the parameter, we should have a note explaining how it might be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah gotcha, yeah a note here would be helpful for clarity. In Ladybird's implementation of these changes, I've actually included a couple of extra parameters to this AO to create useful exception messages, so I'm not sure that unit being treated specially by the spec is all that necessary for implementers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I'll go ahead and close this PR

gibson042 added a commit to gibson042/ecma402 that referenced this pull request Mar 4, 2025
@trflynn89 trflynn89 closed this Mar 4, 2025
@trflynn89 trflynn89 deleted the validatedurationunitstyle branch March 4, 2025 18:14
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