kadircet added a comment. Just writing down my investigation results for context: Looks like we'll never escape `$%'(,/:;?@[^{|}` anymore.
Markdown already doesn't provide backslash escaping for `$%',/:;?@^|` which leaves us with parentheses `([{}`: - `[` looks fine as it is only used for hyperlinks and we escape the `]` if need be - '(' again seems to be only special for providing a text for hyperlinks, so that should also be OK. - `{}` apparently doesn't do anything in normal markdown but some extensions make use of it for adding `id`s to headers, i.e `# header1 {#mycrazyheader}`, so again should be OK. ================ Comment at: clang-tools-extra/clangd/FormattedString.cpp:69 + return false; + if (Contents.front() == '!' || Contents.front() == '?' || + Contents.front() == '/') ---------------- what is the `?` for ? ================ Comment at: clang-tools-extra/clangd/FormattedString.cpp:73 + // Check the start of the tag name. + if (Contents.empty() || !llvm::isAlpha(Contents.front())) + return false; ---------------- we early exited already in case of empty `contents` ================ Comment at: clang-tools-extra/clangd/FormattedString.cpp:130 + case '#': { // ATX heading. + if (!(StartsLine && Before.empty())) + return false; ---------------- nit: if(!StartsLine || !Before.empty())) ================ Comment at: clang-tools-extra/clangd/FormattedString.cpp:132 + return false; + llvm::StringRef Rest = After.ltrim(C); + return Rest.empty() || Rest.startswith(" "); ---------------- nit: After.ltrim('#') ================ Comment at: clang-tools-extra/clangd/FormattedString.cpp:133 + llvm::StringRef Rest = After.ltrim(C); + return Rest.empty() || Rest.startswith(" "); + } ---------------- markdown is weird :( ================ Comment at: clang-tools-extra/clangd/FormattedString.cpp:139 + // ]: is a link reference + // The following are only links if the a link reference exists: + // ] by itself is a shortcut link ---------------- s/the a/the/ ================ Comment at: clang-tools-extra/clangd/FormattedString.cpp:150 + // Not a delimiter if surrounded by space. + return !SpaceSurrounds(); + case '-': // Setex heading, horizontal ruler, or bullet. ---------------- `_` seems to behave different than `*` :( it seems to rather depend on the spaces around the text being emphasized, i.e ``` foo _ bar _ foo -> no emphasis foo _ bar_ foo -> no emphasis foo _bar_ foo -> emphasis on bar foo_bar_ foo -> no emphasis ``` so this should rather be `Before.endswith(" ") && isAlpha(After)` for the beginning of emphasis and the opposite for the ending. Not sure if there's an easy way to decide on it in isolation. ================ Comment at: clang-tools-extra/clangd/FormattedString.cpp:162 + return true; + return !SpaceSurrounds(); + case '<': // HTML tag (or autolink, which we choose not to escape) ---------------- nit: `return IsBullet() || RulerLength() || !SpaceSurrounds()`; ================ Comment at: clang-tools-extra/clangd/FormattedString.cpp:164 + case '<': // HTML tag (or autolink, which we choose not to escape) + return looksLikeTag(After); + case '>': // Quote marker. Needs escaping at start of line. ---------------- is this really worth all the trouble ? ================ Comment at: clang-tools-extra/clangd/FormattedString.cpp:166 + case '>': // Quote marker. Needs escaping at start of line. + return StartsLine && Before.empty(); + case '&': { // HTML entity reference ---------------- I believe it is also allowed to have whitespaces(less than 4?) before the `>` to be interpreted as a quote marker. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75687/new/ https://reviews.llvm.org/D75687 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits