Page MenuHomePhabricator

Test instance: Fix remaining issues with bracket matching
Closed, ResolvedPublic3 Estimated Story Points

Description

Following up on previous investigation into the scope T259700: Investigation: scope bracket matching and tag matching add-ons, debug the behavior of the bracket matching add on in the the 2017 (source view inside VE) and 2010 wikitext editors. These changes are to be implemented on the test instance for usability testing (and to avoid that our branch on the beta cluster does not get blocked by the time required for testing).

See also the initial investigation for more detail: T254976: Investigation: Bracket matching

Requirements:

  • Change highlighting styles. Matched and unmatched brackets should have white text color (Base100). Matched bracket character should have a gray background color (Base20), unmatched brackets should have a red background (Red30). Needs some cleanup, an upstream style is commented out when it should be overridden.
  • Fix bracket-matching logic to consistently detect all types of brackets, in each configuration. Currently (, {, [ each highlight but not in every situation and it's not yet clear what is causing this. Possibly add < as well. Must include tests.
  • Fix VisualEditor "new wikitext" integration breaking the cursorActivity event.

Nice to have:

  • Fix bracket-matching to detect and highlight "{{" and "[[" as pairs.

Mocks:

Bracket matching.png (442×1 px, 62 KB)

Bracket unmatched.png (474×1 px, 56 KB)

Event Timeline

Change 629048 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/CodeMirror@master] [POC] Fix highlighting of matching brackets

https://gerrit.wikimedia.org/r/629048

Lena_WMDE changed the point value for this task from 3 to 5.

Change 629667 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/CodeMirror@master] [POC] Enable bracket matching on VE

https://gerrit.wikimedia.org/r/629667

Change 629668 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/CodeMirror@master] [POC][WIP] Forward selection done in VE to CodeMirror

https://gerrit.wikimedia.org/r/629668

WMDE-Fisch moved this task from Doing to Sprint Backlog on the WMDE-QWERTY-Sprint-2020-09-23 board.
WMDE-Fisch added a subscriber: WMDE-Fisch.
  • Change highlighting styles. Matched and unmatched brackets should have white text color (Base100). Matched bracket character should have a gray background color (Base20), unmatched brackets should have a red background (Red30). Needs some cleanup, an upstream style is commented out when it should be overridden.

Done in https://gerrit.wikimedia.org/r/629048

  • Fix VisualEditor "new wikitext" integration breaking the cursorActivity event.

Done in https://gerrit.wikimedia.org/r/629667 and https://gerrit.wikimedia.org/r/629668

Still needs to be but on GitLab and deployed though.

cd CodeMirror
git fetch --all
git checkout master
git pull
git checkout -b wmde-alpha-deploy

git fetch "https://gerrit.wikimedia.org/r/mediawiki/extensions/CodeMirror" refs/changes/48/629048/1
git branch highlight-style-fix FETCH_HEAD

git fetch "https://gerrit.wikimedia.org/r/mediawiki/extensions/CodeMirror" refs/changes/68/629668/4
git branch forward-selection FETCH_HEAD

git merge highlight-style-fix forward-selection
git push -u gitlab highlight-style-fix forward-selection wmde-alpha-deploy

image.png (541×1 px, 120 KB)

To deploy, I had to port the other POC branches over to gitlab. Then finally,

cd CodeMirror
sudo git remote add gitlab https://gitlab.com/wmde/mediawiki-extensions-CodeMirror.git

It works! Demo instructions: Preferences -> Beta features -> check "New wikitext mode". Edit source.

The "<" angle brackets aren't matched, but that was a nice-to-have and for tags, tag-matching will be much more important anyway.

To fix: an unmatched bracket between two matched brackets breaks the matched brackets.

Screenshot from 2020-10-06 13-50-08.png (182×345 px, 15 KB)

Screenshot from 2020-10-06 13-50-30.png (152×240 px, 8 KB)

Lena_WMDE changed the point value for this task from 5 to 3.Oct 7 2020, 8:10 AM
awight added a subscriber: awight.

Ready to demo. There are still some edge cases, but I'm not sure whether they're marginal or need to be fixed now. For example, putting the cursor directly on a bracket will now show you whether or not it's matched, regardless of intervening unrelated punctuation. But if the cursor is between "{" and "]", it will show both brackets as unmatched. This seems like a reasonable compromise, otherwise please recommend how to fix.

Lena_WMDE claimed this task.
Lena_WMDE moved this task from Demo to Done on the WMDE-QWERTY-Sprint-2020-10-07 board.

Change 629048 abandoned by Thiemo Kreuz (WMDE):
[mediawiki/extensions/CodeMirror@master] [POC] Fix highlighting of matching brackets

Reason:
Now part of Ib01d991.

https://gerrit.wikimedia.org/r/629048

Change 629667 abandoned by Thiemo Kreuz (WMDE):
[mediawiki/extensions/CodeMirror@master] [POC] Enable bracket matching on VE

Reason:
Now part of Ic403e0a.

https://gerrit.wikimedia.org/r/629667

Change 629668 abandoned by Thiemo Kreuz (WMDE):
[mediawiki/extensions/CodeMirror@master] [POC] Forward selection done in VE to CodeMirror

Reason:
Now part of Ic403e0a.

https://gerrit.wikimedia.org/r/629668