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

Add Markdown plugin #890

Merged
merged 21 commits into from
Dec 15, 2020
Merged

Add Markdown plugin #890

merged 21 commits into from
Dec 15, 2020

Conversation

igordsm
Copy link
Contributor

@igordsm igordsm commented Oct 2, 2020

This fixes #850 . Add a Markdown plugin with the following shortcuts:

  • Ctrl+b for bold
  • Ctrl+Shift+I for italic
  • Ctrl+K for links

See a demo below. All three works with and without a selection (and place the cursor where it makes sense).

PR-markdown

@cassidyjames
Copy link
Contributor

Ah, yeah Ctrl+I is the jump-to-line feature. Hrm. @danrabbit thoughts on that?

@cassidyjames cassidyjames requested a review from a team October 8, 2020 20:43
Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline comments - review for functionality and regressions to follow.

plugins/markdown-actions/markdown-actions.vala Outdated Show resolved Hide resolved
@jeremypw
Copy link
Collaborator

I think all the shortcuts should work with <Control><Shift> otherwise its confusing whether or not you need to hold the shift key down.

Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code works as expected and no regressions were found. Some coding issues are mentioned inline.

plugins/markdown-actions/markdown-actions.vala Outdated Show resolved Hide resolved
@igordsm igordsm requested review from jeremypw and removed request for a team October 24, 2020 13:19
@igordsm
Copy link
Contributor Author

igordsm commented Oct 24, 2020

@jeremypw Thank you for the reviews! I have addressed all but one (left a comment in the discussion above) and re-request a review.

I actually like using ctrl+shift more, since it feels more natural to apply the markups after selecting text.

Edit: I am not sure how I removed elementary/ux from the review. Sorry about that.

jeremypw
jeremypw previously approved these changes Oct 26, 2020
@jeremypw
Copy link
Collaborator

Works nicely now - thank! But you will need to add an entry to the next release in the appdata file. I'll reinstate the review request to the ux team.

@jeremypw jeremypw requested a review from a team October 26, 2020 16:37
@igordsm
Copy link
Contributor Author

igordsm commented Oct 27, 2020

By the way, the Flatpak CI is returning with a very non-specific error (Failed to download sources: module gtksourceview: Failed to execute child process ?git? (No such file or directory)) that appears to be unrelated to the last commit. Is it supposed to work?

Edit: Nevermind. Works OK now.

@igordsm
Copy link
Contributor Author

igordsm commented Nov 21, 2020

I have created a standalone repository https://github.com/igordsm/code-markdown-plugin that implements this PR and installs to the correct folders in Elementary 6.

I started a discussion on third party plugins at #908 . I would be happy to hear your @jeremypw @cassidyjames thoughts about that.

Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to work as expected except with undo. It looks like for some reason undo history sees either 2 or 3 actions. For example:

  1. Create some text foo
  2. Highlight and Ctrl + Shift+ B for bold
  3. Notice that the text is now **foo** as expected
  4. Press Ctrl + Z for undo
  5. Notice that the text is now **foo instead of foo

with links you'll notice that each bracket and the set of parentheses are treated individually

@davidmhewitt
Copy link
Member

davidmhewitt commented Nov 25, 2020

It looks like for some reason undo history sees either 2 or 3 actions

@igordsm If I remember correctly, the solution to this is to call TextBuffer.begin_user_action () at the start of an "action" (e.g. making something bold), then do all of the insertions and deletions necessary, then call TextBuffer.end_user_action (). The undo manager should then treat that as one action.

@igordsm
Copy link
Contributor Author

igordsm commented Nov 28, 2020

@davidmhewitt Thank you. Just done that and it works as expected. @danrabbit undo/redo now work as expected.

code-markdown-undo

Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected! Nice job

@danirabbit danirabbit merged commit 1868f07 into elementary:master Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WYSIWYG-like keyboard shortcuts in Markdown
6 participants