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.

[aphillips]

@mihnita, are you satisfied by my/@eemeli's replies? Or do you want to discuss it and just haven't had time to come back to this issue? Would you be okay moving it to v48?

[mihnita]

I'm OK moving it to v48.

The answers are clear, thank you Eemeli.

But I think it would be good to have a name other than "default", I explained why.
Not like I have a good name right now though.

---

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.

I think it would be good to define it, if we want various implementations to give us similar results.
Unless there are some arguments against specifying it (like "excessive burden on implementation" or whatever).

---

If you prefer I can open two specific tickets for these items.

[aphillips]

resolved in the 2025-02-24 call to make the default strategy required for implementation as the default. No change to the name. PR needed