Allow symbols to be escaped in DocText #3375
Conversation
rmhartog
requested review from
bterlson,
markcowl,
allenjzhang and
timotheeguerin
as code owners
|
@microsoft-github-policy-service agree |
rmhartog
force-pushed
the
bug/escape-characters-in-docblock
branch
from
bf80d93
to
2408c62
Compare
There was a problem hiding this comment.
@rmhartog thanks for the contribution, we'll discuss if that's something we want to add to the grammar of js doc. Seeing that it does that in TypeSCript I would assume it shouldn't be a problem. In the meantime if you want this to work you can also escape by wrapping the @ in backtick(markdown) playground
|
/azp run typespec - pr tools |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
❌ There is undocummented changes. Run The following packages have changes but are not documented.
Show changes |
|
You can try these changes at https://cadlplayground.z22.web.core.windows.net/prs/3375/ Check the website changes at https://tspwebsitepr.z22.web.core.windows.net/prs/3375/ |
| @@ -1036,6 +1042,14 @@ export function createScanner( | |||
| return text; | |||
| } | |||
|
|
|||
| function getDocTextValue(): string { | |||
| if (tokenFlags & TokenFlags.Escaped) { | |||
| return unescapeString(tokenPosition, position); | |||
There was a problem hiding this comment.
We talked and I think we should have a different escape logic for the doc comment to be equivalent to JSDoc
Currently with your implementation it is erroring on invalid ecsape See this playground which is the logic we want for string but this would not be the same behavior as jsdoc(and also potentially a breaking change)
So we think if the backslash doesn't escape anything special it should just ignore and include it as is
/** Some \i escaped\*/
should result in Some \i escaped\ string value
There was a problem hiding this comment.
I've added logic to ignore invalid escape sequences and return them verbatim, is this the direction you'd like to go?
There was a problem hiding this comment.
I've inlined the unescapeString logic for this specific case and it now only handles \@ as an escape sequence. I'm unsure what other escape sequences would need to be supported within doctext, and also if this needs to be moved to a separate function. Curious to hear your thoughts.
There was a problem hiding this comment.
I think this is nearly there, there is one edge case that is still not correct, if the escape is the last char
/** some \i \@escape \*/
model Foo {}
here i would espect the doc text to be
some \i @escape \
but is
some \i @escape \*
could you add those ignored escape to the test case as well
rmhartog
force-pushed
the
bug/escape-characters-in-docblock
branch
from
d5871e6
to
4306a71
Compare
rmhartog
force-pushed
the
bug/escape-characters-in-docblock
branch
from
4306a71
to
c1e853c
Compare
|
/azp run typespec - pr tools |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Related to #3374, this is an implementation of escaping characters in doc comments, such as the
@symbol.