The spec/bidi.json is "overreaching"?

[mihnita]

I am working on the bidi support, and I have some doubts about the following test cases in spec/bidi.json:

{
  "defaultTestProperties": {
    "bidiIsolation": "default",
    "locale": "en-US"
  },
  "tests": [
    ...
    {
        "description": "complex-message   = o *(declaration o) complex-body o",
        "src": "\u200E .local $x = {1} {{ {$x}}}",
        "exp": " \u20681\u2069"
    },
    {
        "description": "complex-message   = o *(declaration o) complex-body o",
        "src": ".local $x = {1} \u200F {{ {$x}}}",
        "exp": " \u20681\u2069"
    },
    {
        "description": "complex-message   = o *(declaration o) complex-body o",
        "src": ".local $x = {1} {{ {$x}}} \u2066",
        "exp": " \u20681\u2069"
    },
    ...
  ]
}

I suspect that a test without any kind of bidi control characters in the source would not have any bidi control characters in the expected output:

    {
        "description": "complex-message   = o *(declaration o) complex-body o",
        "src": ".local $x = {1} {{ {$x}}}",
        "exp": " 1"
    },

Question: is that the case?
Or will the "exp" still be " \u20681\u2069" because of "bidiIsolation": "default"?

What does "bidiIsolation": "default" actually means?
Does it mean "get it from the locale?"
What are the valid values for bidiIsolation other then default and none?

---

Now, the whole thing looks under-specified to me.

Depending on the answer to the above I might have a real issue, or "all good, but need better documentation".

Thank you,
M.

[eemeli]

The test/README.md should probably be updated with some documentation on the bidiIsolation that was added in #917; right now its values are only cursorily explained in the test schema.

What does "bidiIsolation": "default" actually means?

It means that the Default Bidi Strategy is applied. That's what's producing the FSI/PDI isolates in the output.

[mihnita]

The test/README.md should probably be updated with some documentation on the bidiIsolation that was added in #917; right now its values are only cursorily explained in the test schema.

Hm... then bidiIsolation becomes another external parameter that one must (potentially) add to a public API. Similar to locale.
That would require API changes in ICU (not a major problem, but that takes time, review, etc)

1. I think that "default" is confusing here

When see in it the mind goes to something like an enum + one of them being default.
As in date format can be [ narrow, short, long, full, default ] (and "default" is also "what happens when nothing is specified")
Maybe a bad practice, but not uncommon: DateFormat.DEFAULT

So if you agree I can think of a better name. Or maybe you have one.

2. Regardless, we should define what is a default :-)

As in, if I don't specify an bidiIsolation when I create a message, what will be the value used? What will happen? Will we do nothing (bidiIsolation=none), or will we apply the Default Bidi Strategy (bidiIsolation=whatever_name_we_decide_to_give_it_that_is_less_confusing :-)

For example in case of a missing locale the default will be used (with something like Locale.getDefault()). (and see, another confusion with "default" meaning something that can actually change)

[eemeli]

Hm... then bidiIsolation becomes another external parameter that one must (potentially) add to a public API. Similar to locale.

Yes. This requirement was added in #315, which was accepted and merged in June 2023, after a little over 7 months' review.

So if you agree I can think of a better name. Or maybe you have one.

The original name for this was "compatibility", and was renamed to "default" in #566. If you'd like to propose a different name, the comments on that PR should probably be reviewed first; there's probably some relevant meetings notes on this as well.

As in, if I don't specify an bidiIsolation when I create a message, what will be the value used? What will happen?

That's up to the implementation.