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

Describing @direction #276

Merged
merged 18 commits into from
Oct 10, 2019
Merged

Describing @direction #276

merged 18 commits into from
Oct 10, 2019

Conversation

gkellogg
Copy link
Member

@gkellogg gkellogg commented Oct 1, 2019

Note that examples won't build properly until it's fully supported in the Ruby implementation.

For #11.


Preview | Diff

@gkellogg gkellogg requested review from pchampin and iherman October 1, 2019 20:53
@gkellogg
Copy link
Member Author

gkellogg commented Oct 1, 2019

cc/ @r12a and @aphillips. The text references String Meta and riffs off of that content. It likely also may cause you to want to update that document based on the direction (sic) laid out here.

@aphillips
Copy link

@gkellogg Thanks! I'll have a look at this PR shortly. String-Meta definitely is in need of an update per our various conversations; I'll try to start in on that as well.

Copy link
Member

@iherman iherman left a comment

Choose a reason for hiding this comment

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

  • I think example 76 is incomplete: @direction should either be set to "ltr" or to null for the author; at present, it would set an "rtl" on an english text.
  • We should get the reaction of @aphillips or @r12a, but I believe the official terminology is "base direction". Maybe we should adopt it.
  • Where do you intend to put the two RDF mapping possibilities? The current document just says that the direction will be dropped, which is of course one option.
  • The text under example 75 got it wrong for the display of the Arabic title; it is not correctly displayed (I let @r12a give you the correct HTML encoding. My experience is that putting such texts into a separate paragraph that is set to rtl is safe; it would actually put the title into a separate, right-justified line, which may make things clearer)

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@gkellogg
Copy link
Member Author

gkellogg commented Oct 2, 2019

  • I think example 76 is incomplete: @direction should either be set to "ltr" or to null for the author; at present, it would set an "rtl" on an english text.

👍

  • We should get the reaction of @aphillips or @r12a, but I believe the official terminology is "base direction". Maybe we should adopt it.

👍

  • Where do you intend to put the two RDF mapping possibilities? The current document just says that the direction will be dropped, which is of course one option.

Those details go in the API document. The text references the Deserialize JSON-LD to RDF Algorithm for a description of how base direction can be preserved using non-standardized mechanisms. Do you think we need to detail them in the syntax document?

  • The text under example 75 got it wrong for the display of the Arabic title; it is not correctly displayed (I let @r12a give you the correct HTML encoding. My experience is that putting such texts into a separate paragraph that is set to rtl is safe; it would actually put the title into a separate, right-justified line, which may make things clearer)

Other than the use of <em> vs <span> it's exactly the same markup as is used in the string-meta document. If I change it to a <div> it works okay, but messes up the flow. Perhaps @aphillips or @r12a can suggest a remedy the preserves flow.

Interestingly, if I add exactly the same markup here, it seems to also mess up (but differently). HTML و CSS: تصميم و إنشاء مواقع الويب. Adding it to a new paragraph preserves order okay:

HTML و CSS: تصميم و إنشاء مواقع الويب

@aphillips
Copy link

Interestingly, if I add exactly the same markup here, it seems to also mess up (but differently). HTML و CSS: تصميم و إنشاء مواقع الويب. Adding it to a new paragraph preserves order okay:

HTML و CSS: تصميم و إنشاء مواقع الويب

Regarding "it seems to mess up": the wrapping behavior is actually correct, if you think in terms of being an Arabic speaker--in an RTL language, the start of the string is on the right side (and notice the colon goes to the left of "CSS"). When I saw that this happened in the preview, I thought it was amusing (since it illustrates the need for bidi support) but also confusing if you don't understand what's going on. This is one reason why we tend to put the examples on lines by themselves (for clarity to folks with "left-to-right eyes"): we have to explain that the "brokenness" is because left-to-right readers are trained to expect the LTR token to start the line ("because it is next")--it's a form of confusion about where "start" is.

Ultimately, what you're experiencing is kind of the point of all this. Text that was previously broken in Arabic now reads correctly. It's instructive to have the reverse experience, I think. Here's a google translate of the "Interestingly" quote at the start of this comment (with an LTR span around the title):

ومن المثير للاهتمام ، إذا أضفت نفس الترميز تمامًا هنا ، فيبدو أن الأمر أيضًا يفسد (ولكن بشكل مختلف). HTML و CSS: web design and creation. إضافة إلى فقرة جديدة يحافظ على النظام بخير

image

Anyway, use one line for legibility...

@gkellogg
Copy link
Member Author

gkellogg commented Oct 2, 2019

@aphillips I found in your document in 4.1.1, the string is represented using <span lang="ar" dir="rtl">HTML و CSS: تصميم و إنشاء مواقع الويب</span> (HTML و CSS: تصميم و إنشاء مواقع الويب), which shows up to me with the Arabic part, followed by ":CSS , HTML". But, in #276 (comment), the exact same span shows as "HTML" followed by the Arabic part, followed by ":CSS ," So, the exact same span renders differently in different contexts.

But, if you think what we have in our doc looks correct, then I won't worry about it.

@iherman
Copy link
Member

iherman commented Oct 3, 2019

But, if you think what we have in our doc looks correct, then I won't worry about it.

I ran in the same problem in the PR on the publication manifest. I think that, for readability's sake, putting the problematic sentences into a separate paragraph is better. This is what I did in, e.g., section 2.4.4.1 and section 2.6.2. I agree with @aphillips that keeping such examples on separate lines is more readable

@iherman
Copy link
Member

iherman commented Oct 3, 2019

Those details go in the API document. The text references the Deserialize JSON-LD to RDF Algorithm for a description of how base direction can be preserved using non-standardized mechanisms. Do you think we need to detail them in the syntax document?

I am not sure. On the one hand, it would probably make the syntax doc too convoluted, on the other hand, as I said many times, the JSON-LD user will tend to read the syntax document only. I guess once the API document is done in this respect, at the minimum a reference would be a good idea, but I could also see the value of having an informative appendix in the syntax document.

@gkellogg
Copy link
Member Author

gkellogg commented Oct 3, 2019

But, if you think what we have in our doc looks correct, then I won't worry about it.

I ran in the same problem in the PR on the publication manifest. I think that, for readability's sake, putting the problematic sentences into a separate paragraph is better. This is what I did in, e.g., section 2.4.4.1 and section 2.6.2. I agree with @aphillips that keeping such examples on separate lines is more readable

Okay, I'll take care of that tomorrow. It is a bit disconcerting that the inline experience can be so variable.

@gkellogg
Copy link
Member Author

gkellogg commented Oct 3, 2019

But, if you think what we have in our doc looks correct, then I won't worry about it.

I ran in the same problem in the PR on the publication manifest. I think that, for readability's sake, putting the problematic sentences into a separate paragraph is better. This is what I did in, e.g., section 2.4.4.1 and section 2.6.2. I agree with @aphillips that keeping such examples on separate lines is more readable

Okay, I'll take care of that tomorrow. It is a bit disconcerting that the inline experience can be so variable.

@iherman I instead put it as a statements table, which shows the directionality properly within table cells; this is consistent with how we treat other examples, see what you think.

@gkellogg gkellogg requested a review from iherman October 3, 2019 23:26
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@gkellogg gkellogg changed the title First stab at describing @direction in the syntax document. Describing @direction Oct 4, 2019
index.html Show resolved Hide resolved
@aphillips
Copy link

aphillips commented Oct 4, 2019

With the exception of my one comment, the latest version looks good to me.

@iherman iherman mentioned this pull request Oct 4, 2019
@iherman
Copy link
Member

iherman commented Oct 4, 2019

This issue was discussed in a meeting.

  • RESOLVED: the WG is happy to merge #276, but leaves it to the editors whether they do it now or after the API changes is also in a PR
  • ACTION: create issue on values of language maps not including <code>@direction</code> (Gregg Kellogg)
View the transcript Benjamin Young: See Issue Syntax#1§1
Benjamin Young: See Pending PR on @direction
Benjamin Young: no much to discuss; just to make sure everyone is aware of the PR
Gregg Kellogg: I will only merge it when the tests pass;
… currently working on my implementation;
… maybe only merge when we have an API PR.
… The syntax PR does not address the ability for a language map to contain value objects with @value and @direction.
… Currently they only support strings.
… So there is no way to override the direction in a language map.
Ivan Herman: I think we can defer this to a future version.
Benjamin Young: is there an issue for this already?
Ivan Herman: raise an issue, so we can make it clear if we decide to postpone it.
Action #3: create issue on values of language maps not including @direction (Gregg Kellogg)
Dave Longley: the syntax doc proposes 2 or 3 ways to represent this in RDF in order to roundtrip.
… We should only recommend one.
… We would have to maintain them for many years.
… My preferred option would be to drop the @direction by default,
… with an option to serialize it in the way we think is the best for future RDF.
… That would force people to update their RDF lib,
… but that what we were expecting if a dedicated WG had been created.
Ivan Herman: half of your wishes are already fulfilled:
… by default, the @direction is dropped.
… We picked two options for roundtripping:
… * the i18n family of datatypes,
… * the “compound literal” approach (not a very good name).
… At the meeting in Fukuoka, even around the table,
… there was no clear consensus about which one was the best option.
… The idea is to let the community decide which one they want to use.
Dave Longley: Why not a 3rd option where we add a property to a literal?
Ivan Herman: because this is not valid RDF.
… The failure to create a WG shows that the RDF community is not willing to change the core of RDF.
… The advantage of the two proposed approaches is that they work with the deployed RDF infrastructure,
… even though they require some application-level knowledge to be used.
Dave Longley: What I’m hearing is “more technical depts, and no interop”
… If this is something that people need, and we introduce a hack to get it work,
… people will adopt the hack.
Ivan Herman: you can call it a hack, but this is the only thing that people seem ready to accept.
… Trying to force RDF to change is a bigger hack in my opinion.
Dave Longley: JSON-LD 1.0 did influence the RDF WG at the time, to extend RDF.
Ivan Herman: this is the big difference; there was an RDF WG at the time.
… There is none at the moment, nor can I foresee one in the near future.
Dave Longley: sometimes, standard follow implementations, rather than the opposite.
Pierre-Antoine Champin: I’d argue that the proposed solutions are not so hacky from and RDF point of view
… I prefer the datatype solution
… but none of them seem a scandal to me
… if there was an RDF WG working on this, they would consider one of these as the future standard
Ivan Herman: +1 to pchampin
Gregg Kellogg: I agree, these solutions are reasonable from the point of view of RDF.
… Also, the default option is to drop direction.
… The majority of data will not use direction, and not produce RDF differently than today.
Dave Longley: dropping it is a good default;
… but I expect Digital Bazaar being force to keep it in some way.
… For example, in VC, when the graph is signed, we need to keep the whole information.
Gregg Kellogg: from a digital signature point of view,
… I would not consider the direction to be relevant.
… Would two values differing only by direction be really different?
Dave Longley: this is scary; that could be turned into a terrible vulnerability.
Ivan Herman: I would like a resolution to allow Greg to merge the direction PR;
… this has lingered for too long.
Proposed resolution: the WG is happy to merge #276, but leaves it to the editors whether they do it now or after the API changes is also in a PR (Ivan Herman)
Ivan Herman: +1
Gregg Kellogg: +1
Harold Solbrig: +1
Pierre-Antoine Champin: +1
Benjamin Young: +1
Tim Cole: +1
Dave Longley: +0
Ruben Taelman: +1
Resolution #2: the WG is happy to merge #276, but leaves it to the editors whether they do it now or after the API changes is also in a PR

@gkellogg gkellogg merged commit 5b7e06c into master Oct 10, 2019
@gkellogg gkellogg deleted the text-direction branch October 10, 2019 21:55
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.

5 participants