Skip to content

Fix use native types flag with non-native values#671

Open
niklasl wants to merge 7 commits into
w3c:mainfrom
niklasl:fix-use-native-types-issue-670
Open

Fix use native types flag with non-native values#671
niklasl wants to merge 7 commits into
w3c:mainfrom
niklasl:fix-use-native-types-issue-670

Conversation

@niklasl

@niklasl niklasl commented Nov 18, 2025

Copy link
Copy Markdown
Contributor

This addresses #670.

  • Add test case
  • Implement fix in algorithm

Preview | Diff

@niklasl

niklasl commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

Unless there is a better idea for refactoring the spec algorithm in this place (see #670), I'll probably implement the fix similarly to how I'll fix it in TRLD: niklasl/trld@a66feee (i.e. add a done converting flag).

@niklasl

niklasl commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

I made a possibly cleaner approach in niklasl/trld@5fe0981 (explained in that commit message). Any feedback welcome; I'll probably proceed with fixing the spec algorithm accordingly in the coming week.

@niklasl niklasl force-pushed the fix-use-native-types-issue-670 branch from 5185051 to ad93494 Compare June 5, 2026 22:41
@niklasl niklasl marked this pull request as ready for review June 6, 2026 14:12
@niklasl

niklasl commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

I've now implemented what I see as an acceptable fix, both in TRLD: niklasl/trld#5 (which now passes all fromRdf tests including the one added in this PR); and as a proposed spec change in this PR.

It is a slight refactoring of the RDF to Object Conversion algorithm, to address this issue whilst also keeping or seeking to improve the clarify of the algorithm.

Comment thread index.html Outdated
Comment thread index.html
@niklasl niklasl moved this to PRs in JSON-LD Management Jun 10, 2026
@niklasl niklasl force-pushed the fix-use-native-types-issue-670 branch from 8e1a6c7 to f028c25 Compare June 17, 2026 15:56
Comment thread index.html Outdated
Comment thread index.html Outdated
@BigBlueHat BigBlueHat moved this from PRs to Discuss-Call in JSON-LD Management Jun 24, 2026
@BigBlueHat BigBlueHat moved this from Discuss-Call to PRs in JSON-LD Management Jun 24, 2026
@w3cbot

w3cbot commented Jun 24, 2026

Copy link
Copy Markdown

This was discussed during the #json-ld meeting on 24 June 2026.

View the transcript

Pull Request 671 Fix use native types flag with non-native values (by niklasl) [needs discussion]

niklasl: this issue has been around forevever; I think that implementations do not follow the spec (they would encounter issues),
… but instead implement the suggested change in this PR
… note that the HTML diff is easier to follow than the PR diff

bigbluehat: I agree on the separate PR for the change on the "quote" thing

<Zakim> pchampin, you wanted to react to anatoly-scherbakov

pchampin: the CI issue is due to an problem I created with common files,
… which w3c/json-ld-api#693 (not yet merged) is supposed to fix

<gb> Pull Request 693 fix bug in Rakefile after removing common/ (by pchampin)


Comment thread index.html Outdated
@niklasl

niklasl commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Oh my, the github bug that changes newline form struck again. (It seems to occur, sometimes, when a suggestion is applied through the web interface; possibly if there is some mix of line ending forms already in the doc.) I'll take a look, and importantly rework the last commit (and force-push that).

@niklasl niklasl force-pushed the fix-use-native-types-issue-670 branch from 840e046 to 8bccd8a Compare June 26, 2026 11:35
niklasl and others added 4 commits June 26, 2026 13:39
Test use native types flag with non-native values.
When useNativeTypes is true, the else clause was never reached even if a
native type couldn't be used; leaving only plain strings where e.g.
language-tagged string literals were the source.

This is a slight refactoring of the RDF to Object Conversion algorithm,
to address this issue, whilst keeping or improving the clarify of the
algorithm.

Fixes w3c#670
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
niklasl and others added 3 commits June 26, 2026 13:39
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Pierre-Antoine Champin <github-100614@champin.net>
@niklasl niklasl force-pushed the fix-use-native-types-issue-670 branch from 8bccd8a to 39641ad Compare June 26, 2026 11:39
@niklasl

niklasl commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

OK; the change this PR proposes looks clean again now (the culprit was \r\n-line-endings from b028727; itself a change suggestion).

I recommend squash-and-merge for when we merge this, to have a clean change commit for this in the main branch.

@TallTed

TallTed commented Jun 26, 2026

Copy link
Copy Markdown
Member

the culprit was \r\n-line-endings from b028727; itself a change suggestion

That change suggestion came from me.

It appears that GitHub has badly tweaked something in how it handles such change suggestions. I certainly didn't intend to change the line endings, and I can find nothing about that commit that indicates the line endings were being changed.

If anyone figures out a switch that I can flip to avoid such issues going forward, please let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

6 participants