Skip to content
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

[WIP] feat: support underline for markdown and update string format pattern #9245

Closed
wants to merge 173 commits into from

Conversation

Bad3r
Copy link
Collaborator

@Bad3r Bad3r commented Apr 26, 2023

Update formatting functions and format-text! to use %s pattern

TODO

  • use util/get-selected-text

Depends on:

breaking change:

  • Remove /underline command and replace it with modKey + u shortcut

issues:

forum posts:

QA Testing Instructions for Text Formatting Feature

Repeat the steps for markdown and org-mode formats.

Initial Setup:

  1. Create a new block with the sentence: LOGSEQ: Logical Organization Graphical-System for Efficient Querying.

Test for Text Formatting Features:

Bold:

  1. Select the word Graphical and apply bold formatting ($MOD+b)
  2. Expected Result: **Graphical**|-System
  3. Verify the result and cursor (|) position.
    Select the styled word Graphical again and remove the formatting by pressing the same shortcut.

Italic:

  1. Select the word Graphical and apply italic formatting ($MOD+i)
  2. Expected Result: *Graphical*|-System
  3. Verify the result and cursor (|) position.
    Select the styled word Graphical again and remove the formatting by pressing the same shortcut.

Underline:

  1. Select the word Graphical and apply underline formatting ($MOD+u)
  2. Expected Result: <u>Graphical</u>|-System
  3. Verify the result and cursor (|) position.
    Select the styled word Graphical again and remove the formatting by pressing the same shortcut.

Strikethrough:

  1. Select the word Graphical and apply the strikethrough formatting ($MOD+Shift+s).
  2. Expected Result: ~~Graphical~~|-System
  3. Verify the result and cursor (|) position.
    Select the styled word Graphical again and remove the formatting by pressing the same shortcut.

Sequential Application and Removal of Formatting:

  1. Select the word Graphical and apply all the text formatting shortcuts. Example: $MOD+i, $MOD+u, $MOD+Shift+s, $MOD+b.
  2. Expected Result: **~~<u>*Graphical*</u>~~**|-System
  3. Verify the result and cursor (|) position.
  4. Then, remove each formatting one by one in reverse order.
  5. Expected Result: Graphical|-System (Graphical should remain selected)
  6. Expected Result: The word Graphical should remain selected after removing the formatting.

Test for Formatting Without Selection:

  1. In a new block without any text selected, apply each formatting command or shortcut. Example: $MOD+i, $MOD+u, $MOD+Shift+s, $MOD+b.
  2. Expected Result: **~~<u>*|*</u>~~**
  3. Verify the result and cursor position.

Please remember to document any discrepancies between the expected and actual results, along with any error messages or issues encountered during testing.

logseq-string-formatting.mp4

This PR updates the text formatting functions to use a consistent %s pattern for wrapping text with the appropriate formatting markers. The format-text! function has also been updated to accommodate these changes.

  • Updated get-bold, get-italic, get-strike-through, get-underline, and get-highlight functions to use %s pattern
  • Modified format-text! to handle the new %s pattern in the formatting functions
  • Ensured compatibility with existing text formatting behavior

The result of this change is introducing a new feature; underline text using mod + u. Since markdown doesn't support underlining, HTML syntax is needed. Due to the opening and closing tags being different for HTML, there was a need to update the text-format! function to accommodate this case. Instead of writing a special case for underlines, I opted to modify the existing format used to a more malleable one.


Warning
Old PR description

Discord: https://discord.com/channels/725182569297215569/735747000649252894/1100463176270745640

This pull request introduces a new feature that adds a keyboard shortcut for underlining text Mod + u in both Markdown and Org-mode formats. The implementation modifies the get-underline function and introduces the underline-format! function to support the respective underline syntax for each format. In Org-mode, the underscore (_) syntax is used, while in Markdown, the text is wrapped with the <u> and </u> HTML tags.

Since Markdown does not natively support underlines, I opted to use HTML tags as a workaround. This presented a challenge as the opening and closing tags are different. Initially, I considered modifying the format-text! function to handle this case, but doing so could potentially break other features. As a result, I decided to not change it.

This approach may have introduced some code duplication, and there could be a more efficient solution beyond my current understanding of the code. I apologize if this adds extra work to the review process. Please let me know if you have any suggestions for improvements or if further modifications are needed.

All translations have been added and its been added to the shortcuts page.

Video Demo:

composed-grasp-log-topic.mp4

QA Steps:

  • open a markdown and/or org-mode graph
  • select some text
  • use the shortcut Mod + u to underline it

Test unwrapping

  • select wrapped text
  • use the same shortcut to undo the formatting.

@Bad3r Bad3r force-pushed the enhance/underline-shortcut branch from 99d0084 to 9a5f037 Compare April 26, 2023 03:59
@Bad3r Bad3r force-pushed the enhance/underline-shortcut branch from 3b810f8 to 3c38c73 Compare April 27, 2023 02:37
This commit updates the text formatting functions to use a consistent %s pattern for wrapping text with the appropriate formatting markers. The format-text! function has also been updated to accommodate these changes.

- Updated get-bold, get-italic, get-strike-through, get-underline, and get-highlight functions to use %s pattern
- Modified format-text! to handle the new %s pattern in the formatting functions
- Ensured compatibility with existing text formatting behavior
@Bad3r Bad3r changed the title feat: Add underline shortcut for Markdown and Org-mode feat: support underline for markdown and update string format pattern Apr 27, 2023
@Bad3r
Copy link
Collaborator Author

Bad3r commented Apr 27, 2023

@logseq/core, this PR is ready for review. Not sure who to ask. The original code is written by @tiensonqin ~3 years ago.~~ Is this change in pattern OK?

@Bad3r Bad3r changed the title feat: support underline for markdown and update string format pattern [WIP] feat: support underline for markdown and update string format pattern Apr 27, 2023
@Bad3r
Copy link
Collaborator Author

Bad3r commented Apr 27, 2023

setting it as WIP for now as it could be improved to be more efficient.

@Bad3r Bad3r force-pushed the enhance/underline-shortcut branch 2 times, most recently from 149fa1e to c2ed3d6 Compare April 28, 2023 04:53
@Bad3r Bad3r force-pushed the enhance/underline-shortcut branch from c2ed3d6 to 3f2a584 Compare April 28, 2023 05:15
andelf and others added 13 commits May 24, 2023 16:20
…ogseq#9388)

* enhance: only autopair tilda when text is selected

* Enhance/editor string formatting e2e (#48)

* enhance(E2E): Test editor italic, bold, strikethrough and underline.

...

---------

Signed-off-by: Bad3r <bad3r@protonmail.com>
* enhance(E2E): Test editor italic, bold, strikethrough and underline.

* disable underline tests until logseq#9245 is merged

* Fix: position

* enhance: add test with all formatting

* enhance: add tests for editor autopairing

* enhance: add tests for editor autopairing

* add: underline test as fixme as a prereq to logseq#9245

* enhance: break tests into steps

* revert: test steps

---------

Signed-off-by: Bad3r <bad3r@protonmail.com>
@CLAassistant
Copy link

CLAassistant commented May 24, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
11 out of 12 committers have signed the CLA.

✅ andelf
✅ logseq-cldwalker
✅ RCmerci
✅ avelino
✅ giuseppedandrea
✅ tiensonqin
✅ Michele0303
✅ candideu
✅ optimistic5
✅ Bad3r
✅ hasecilu
❌ dependabot[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

@Bad3r Bad3r force-pushed the enhance/underline-shortcut branch from 083e4a8 to 201ce7b Compare May 24, 2023 20:41
src/main/frontend/state.cljs Outdated Show resolved Hide resolved
@Bad3r Bad3r force-pushed the enhance/underline-shortcut branch 2 times, most recently from ba07fe6 to 824250c Compare May 24, 2023 20:52
@cnrpman
Copy link
Collaborator

cnrpman commented Jun 4, 2023

Humm, seems tons of conflicts given the great changes of the dicts
Also the commit history is so awful. Better to do a thorough interactive rebase from the current master

@Bad3r Bad3r force-pushed the enhance/underline-shortcut branch from 824250c to 286eb76 Compare June 5, 2023 18:56
@Bad3r
Copy link
Collaborator Author

Bad3r commented Jun 5, 2023

Humm, seems tons of conflicts given the great changes of the dicts Also, the commit history is so awful. Better to do a thorough interactive rebase from the current master

@cnrpman fixed the conflict. This PR is on hold until #9391 is merged.

@cnrpman cnrpman added the :type/hold Hold this PR. won't merge for now label Jun 6, 2023
@Bad3r
Copy link
Collaborator Author

Bad3r commented Jun 24, 2023

Close due to the team not having time to review large community PRs and to save my time.

@Bad3r Bad3r closed this Jun 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature :type/hold Hold this PR. won't merge for now
Projects
None yet