Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Unexpected interaction between emu-format changing spacing around <ins> and completion records #608

Closed
ptomato opened this issue Sep 12, 2024 · 1 comment · Fixed by #610

Comments

@ptomato
Copy link
Contributor

ptomato commented Sep 12, 2024

When applying emu-format to Temporal there was one instance where the text produced by emu-format didn't pass ecmarkup --lint.

diff --git a/spec/intl.html b/spec/intl.html
index d0005fed..02578d4d 100644
--- a/spec/intl.html
+++ b/spec/intl.html
@@ -1017,7 +1017,7 @@
           1. Let _pattern_ be _format_.[[pattern12]].
         1. Else,
           1. Let _pattern_ be _format_.[[pattern]].
-        1. Let _result_ be <del>? </del><ins>! </ins>FormatDateTimePattern(_dateTimeFormat_, _format_, _pattern_, <del>_x_</del><ins>_xFormatRecord_.[[EpochNanoseconds]]</ins>).
+        1. Let _result_ be <del>?</del> <ins>!</ins> FormatDateTimePattern(_dateTimeFormat_, _format_, _pattern_, <del>_x_</del><ins>_xFormatRecord_.[[EpochNanoseconds]]</ins>).
         1. Return _result_.
       </emu-alg>
     </emu-clause>

Causes this error:

Error: spec/intl.html:1020:54: FormatDateTimePattern returns a Completion Record, but is not consumed as if it does

I'm not sure if the fix should be to format <ins> and <del> differently, or to recognize this formatting as unwrapping the completion record.

This isn't a high priority problem for me, as I've found another way to work around it.

@michaelficarra
Copy link
Member

I'm not sure if the fix should be to format <ins> and <del> differently, or to recognize this formatting as unwrapping the completion record.

I'm fine with fixing ecmarkup to handle this, but thinking as a reviewer, I'd probably rather just see full steps added/removed.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants