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

fix(schema): Add hint to see if dataset name contains a visible character #1879

Merged
merged 4 commits into from
Aug 1, 2024

Conversation

rwblair
Copy link
Member

@rwblair rwblair commented Jul 31, 2024

No description provided.

@rwblair rwblair added the exclude-from-changelog This item will not feature in the automatically generated changelog label Jul 31, 2024
@rwblair rwblair requested a review from effigies July 31, 2024 22:28
@effigies effigies changed the title fix(schema) add hint to see if dataset name contains a visible character fix(schema): Add hint to see if dataset name contains a visible character Jul 31, 2024
@rwblair
Copy link
Member Author

rwblair commented Aug 1, 2024

I messed up the initial regex:

> new RegExp('\\S').test("test")
true
> new RegExp('\\S').test(" ")
false
> new RegExp('\S').test(" ")
false
> new RegExp('\S').test("test")
false
> new RegExp('\S').test("S")
true
>

@effigies
Copy link
Collaborator

effigies commented Aug 1, 2024

Single backslash should work. YAML will not interpolate them:

>>> import yaml
>>> yaml.safe_load(r"\S")
'\\S'

So when you do a double \\ it gets expanded to \\\\ in JSON:

curl -sSL https://bids-specification--1879.org.readthedocs.build/en/1879/schema.json | jq .rules.checks.hints.EmptyDatasetName
{
  "issue": {
    "code": "EMPTY_DATASET_NAME",
    "message": "The Name field of dataset_description.json is present but empty of visible characters.\n",
    "level": "warning"
  },
  "selectors": [
    "path == '/dataset_description.json'",
    "type(json.Name) != 'null'"
  ],
  "checks": [
    "match(json.Name, '\\\\S')"
  ]
}

I would rather use raw backslashes, since YAML will let us. This way the implementation can know that it's been given what we actually want, and do any escaping of them itself when needed: https://github.com/bids-standard/bids-validator/pull/2042

@effigies effigies merged commit fdca7ec into bids-standard:master Aug 1, 2024
24 of 25 checks passed
@effigies effigies added the schema Issues related to the YAML schema representation of the specification. Patch version release. label Aug 1, 2024
@rwblair rwblair deleted the schema/dataset-name-hint branch August 1, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude-from-changelog This item will not feature in the automatically generated changelog schema Issues related to the YAML schema representation of the specification. Patch version release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants