Fix regex in language and locale recognition #490
Conversation
This was first mentioned in github/codeql-action#418 Also, raised in oasis-tcs#488. cc: @lcartey
|
cc @michaelcfanning - this addresses a small bug a user of the |
See oasis-tcs/sarif-spec#490 See #418 Note that this changes the sarif spec file. Unless this change is actually merged in the sarif spec repo, the version used by the action will be slightly different.
See oasis-tcs/sarif-spec#490 See #418 Note that this changes the sarif spec file. Unless this change is actually merged in the sarif spec repo, the version used by the action will be slightly different.
|
Any chance someone could take a look at this? We are already using this change in production. |
| @@ -2354,7 +2354,7 @@ | |||
| "description": "The language of the messages emitted into the log file during this run (expressed as an ISO 639-1 two-letter lowercase culture code) and an optional region (expressed as an ISO 3166-1 two-letter uppercase subculture code associated with a country or region). The casing is recommended but not required (in order for this data to conform to RFC5646).", | |||
| "type": "string", | |||
| "default": "en-US", | |||
| "pattern": "^[a-zA-Z]{2}|^[a-zA-Z]{2}-[a-zA-Z]{2}]?$" | |||
| "pattern": "^[a-zA-Z]{2}(-[a-zA-Z]{2})?$" | |||
There was a problem hiding this comment.
zA [](start = 39, length = 2)
Looks good! Also, we could consider the pattern below. It's two characters shorter and, I think, a little more readable. As if readability in regex could be easily quantified. :)
(?i)^[a-z]{2}(-[a-z]{2})?$
@eddynake, @yongyan-gh, we need to get this change into an rtm.6 revision of the SARIF schema as published in the SDK and schemastore.org.
There was a problem hiding this comment.
@eddynaka, @yongyan-gh, I don't think we pushed these changes into an rtm.6 revision. Let's discuss this offline today, it would be good to close on this issue asap. @aeisenberg, sorry for the delay. We do have all the SARIF errata prepared but the holidays (and a need to rerender all errata in an OASIS-approved template) has introduced delays. We're picking it up again now, however.
|
Just following up with this. Can someone merge this PR? |
See oasis-tcs/sarif-spec#490 See github#418 Note that this changes the sarif spec file. Unless this change is actually merged in the sarif spec repo, the version used by the action will be slightly different.
|
@aeisenberg, I apologize for the delay here, we've had to initiate an actual errata process to accept certain changes. I assume you've long moved forward with what you need for your work. :) The new versions of schema, spec, etc., with these changes will be available in early Oct. |
|
That's fine. We're handling things locally correctly. It would just be good to have these changes available to the entire community. |
|
Hi there, @michaelcfanning is there any update me on the status of this change? Is the new version of the specification available somewhere? If so, we can close this PR. |
|
@dmk42 @michaelcfanning I wonder if we plan to either provide the information requested by @aeisenberg and close without action or resolve the conflicts and merge this PR. Should we create an agenda item for next meeting to discuss? |
|
Just cleaning out my backlog. It doesn't look like this PR will ever be merged. |
This was first mentioned in github/codeql-action#418
Also, raised in #488.
The current regex has an extra
]at the end. This means that schema validation will incorrectly validate strings that should be an error. For example, the following strings are valid according to the regex, but should not be valid:cc: @lcartey