Repository navigation
Clarify error when content mapper extension ends in a built-in extension - #64921
GOVARDHAN CHINTA (govardhanchinta99) wants to merge 2 commits into
Conversation
typescript-automation
Bot
added
For Uncommitted Bug
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
I don't think the .json.gz files are supposed to be updated.
There was a problem hiding this comment.
Thanks for catching that. I removed all of the loc/*.json.gz changes in the latest commit, they are back to what main has. The PR now only touches the diagnostic message, the generated Go and JSON files for it, and the tsoptions code and tests.
The loc/*.json.gz files are produced by the localization pipeline and should not be updated in this PR. Restore them to the versions on main.
| "Private_identifiers_cannot_be_used_in_destructuring_patterns_18064": "Private identifiers cannot be used in destructuring patterns.", | ||
| "Content_mapper_file_extension_0_must_begin_with_a_18065": "Content mapper file extension '{0}' must begin with a '.'.", | ||
| "Content_mapper_file_extension_0_is_a_built_in_extension_and_cannot_be_registered_by_a_content_mapper_18066": "Content mapper file extension '{0}' is a built-in extension and cannot be registered by a content mapper.", | ||
| "Content_mapper_file_extension_0_cannot_be_registered_because_it_ends_in_a_built_in_extension_18066": "Content mapper file extension '{0}' cannot be registered because it ends in a built-in extension.", |
There was a problem hiding this comment.
can we support .const.json and other similar extensions instead? as i suggested in my issue (#64917):
my preferred solution is that patterns like .const.json should be supported, to support content mappers like
const-json-content-mapper
There was a problem hiding this comment.
Thanks for the suggestion. Allowing .const.json would reverse a deliberate rule: commit made content mapper extensions that end in a built-in extension invalid, and compound extensions are still allowed as long as they don't end in one. That looks like a decision for the maintainers, so this PR only fixes the misleading wording. If the team would prefer to allow it, I'm happy to rework this PR.
typescript-automation
Bot
added
For Backlog Bug
Andrew Branch (andrewbranch)
left a comment
Andrew Branch (andrewbranch)
left a comment
There was a problem hiding this comment.
This makes sense to me, thanks!
Fixes #64917
A content mapper extension is rejected when it ends in a built-in extension (HasReservedSourceExtension does a suffix check). For
.const.jsonthe error said it "is a built-in extension", which is misleading, because.const.jsonis not built-in, it is rejected because it ends in.json.This changes the message for TS18066 to:
"Content mapper file extension '{0}' cannot be registered because it ends in a built-in extension."The error code and the check itself are unchanged. I did not add support for.const.json-style extensions, since that looks like a design decision for the team.Changes:
npx hereby generate:diagnosticsTesting: the tsoptions package and the compiler test runner (internal/testrunner) pass, and
hereby check:formatis clean.cc Andrew Branch (@andrewbranch), you worked on content mappers, so I'd appreciate your view on whether this wording works.