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: code "block" detection before showing autocomplete #26023
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ZogStriP
commented
Mar 4, 2024
ZogStriP
commented
Mar 4, 2024
ZogStriP
commented
Mar 4, 2024
ZogStriP
commented
Mar 4, 2024
Welp, I did π Will have a look tomorrow. |
ZogStriP
force-pushed
the
fix-in-code-block-detection
branch
from
March 8, 2024 16:03
162f0d3
to
621a9b0
Compare
ZogStriP
commented
Mar 8, 2024
ZogStriP
commented
Mar 8, 2024
ZogStriP
commented
Mar 8, 2024
ZogStriP
commented
Mar 8, 2024
ZogStriP
commented
Mar 8, 2024
app/assets/javascripts/discourse/tests/acceptance/composer-editor-mentions-test.js
Outdated
Show resolved
Hide resolved
ZogStriP
commented
Mar 8, 2024
app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js
Outdated
Show resolved
Hide resolved
ZogStriP
commented
Mar 8, 2024
app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js
Outdated
Show resolved
Hide resolved
ZogStriP
commented
Mar 8, 2024
ZogStriP
force-pushed
the
fix-in-code-block-detection
branch
from
March 9, 2024 18:08
59a544a
to
8a48270
Compare
ZogStriP
commented
Mar 9, 2024
ZogStriP
commented
Mar 9, 2024
ZogStriP
commented
Mar 9, 2024
ZogStriP
commented
Mar 9, 2024
app/assets/javascripts/discourse/tests/helpers/component-test.js
Outdated
Show resolved
Hide resolved
ZogStriP
force-pushed
the
fix-in-code-block-detection
branch
from
March 9, 2024 18:17
8a48270
to
fa48e09
Compare
CvX
reviewed
Mar 11, 2024
app/assets/javascripts/discourse/tests/acceptance/composer-editor-mentions-test.js
Outdated
Show resolved
Hide resolved
app/assets/javascripts/discourse/tests/helpers/component-test.js
Outdated
Show resolved
Hide resolved
app/assets/javascripts/discourse/tests/unit/lib/autocomplete-test.js
Outdated
Show resolved
Hide resolved
app/assets/javascripts/discourse/tests/unit/lib/autocomplete-test.js
Outdated
Show resolved
Hide resolved
app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js
Outdated
Show resolved
Hide resolved
**TL;DR:** Refactor autocomplete to use async markdown parsing for code block detection. Previously, the `inCodeBlock` function in `discourse/app/lib/utilities.js` used regular expressions to determine if a given position in the text was inside a code block. This approach had some limitations and could lead to incorrect behavior in certain edge cases. This commit refactors `inCodeBlock` to use a more robust algorithm that leverages Discourse's markdown parsing library. The new approach works as follows: 1. Check if the text contains any code block markers using a regular expression. If not, return `false` since the cursor can't be in a code block. 1. If potential code blocks exist, find a unique marker character that doesn't appear in the text. 1. Insert the unique marker character into the text at the cursor position. 1. Parse the modified text using Discourse's markdown parser, which converts the markdown into a tree of tokens. 1. Traverse the token tree to find the token that contains the unique marker character. 1. Check if the token's type is one of the types representing code blocks ("code_inline", "code_block", or "fence"). If so, return `true`, indicating that the cursor is inside a code block. Otherwise, return `false`. This algorithm provides a more accurate way to determine the cursor's position in relation to code blocks, accounting for the various ways code blocks can be represented in markdown. To accommodate this change, the autocomplete `triggerRule` option is now an async function. The autocomplete logic in `composer-editor.js`, `d-editor.js`, and `hashtag-autocomplete.js` has been updated to handle the async nature of `inCodeBlock`. Additionally, many of the tests have been refactored to handle async behavior. The test helpers now simulate typing and autocomplete selection in a more realistic, step-by-step manner. This should make the tests more robust and reflective of real-world usage. This is a significant refactor that touches multiple parts of the codebase, but it should lead to more accurate and reliable autocomplete behavior, especially when dealing with code blocks in the editor. > Written by an π€ LLM. Edited by a π§βπ» human.
ZogStriP
force-pushed
the
fix-in-code-block-detection
branch
from
March 11, 2024 16:22
fa48e09
to
472f719
Compare
ZogStriP
commented
Mar 11, 2024
ZogStriP
commented
Mar 11, 2024
ZogStriP
commented
Mar 11, 2024
CvX
approved these changes
Mar 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL;DR: Refactor autocomplete to use async markdown parsing for code block detection.
Previously, the
inCodeBlock
function indiscourse/app/lib/utilities.js
used regular expressions to determine if a given position in the text was inside a code block. This approach had some limitations and could lead to incorrect behavior in certain edge cases.This commit refactors
inCodeBlock
to use a more robust algorithm that leverages Discourse's markdown parsing library.The new approach works as follows:
If not, return
false
since the cursor can't be in a code block.If so, return
true
, indicating that the cursor is inside a code block.Otherwise, return
false
.This algorithm provides a more accurate way to determine the cursor's position in relation to code blocks, accounting for the various ways code blocks can be represented in markdown.
To accommodate this change, the autocomplete
triggerRule
option is now an async function.The autocomplete logic in
composer-editor.js
,d-editor.js
, andhashtag-autocomplete.js
has been updated to handle the async nature ofinCodeBlock
.Additionally, many of the tests have been refactored to handle async behavior. The test helpers now simulate typing and autocomplete selection in a more realistic, step-by-step manner. This should make the tests more robust and reflective of real-world usage.
This is a significant refactor that touches multiple parts of the codebase, but it should lead to more accurate and reliable autocomplete behavior, especially when dealing with code blocks in the editor.