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

feat(editor/copy-paste): refactor copy/export in cljs #8530

Merged
merged 32 commits into from Feb 20, 2023

Conversation

RCmerci
Copy link
Contributor

@RCmerci RCmerci commented Feb 6, 2023

move copy-as related codes from mldoc to cljs

  • copy as text
  • copy as opml
  • copy as html
  • old options: indent-styles, remove-emphasis, remove-page-ref-brackets
  • add option: remove #tag
  • replace block-reference & block-embed & page-embed

--
Closes #6351

@Bad3r Bad3r self-requested a review February 13, 2023 04:38
@Bad3r
Copy link
Collaborator

Bad3r commented Feb 14, 2023

@RCmerci did you test assets export? what is the expected behaviour for local graph assets when exporting?

@RCmerci
Copy link
Contributor Author

RCmerci commented Feb 15, 2023

did you test assets export? what is the expected behaviour for local graph assets when exporting?
this export/copy feature only handles the pages/journals

@Bad3r Bad3r mentioned this pull request Feb 15, 2023
2 tasks
@Bad3r
Copy link
Collaborator

Bad3r commented Feb 16, 2023

this export/copy feature only handles the pages/journals
what is the expected behaviour if a page/journal contain local assets such as images?

@RCmerci
Copy link
Contributor Author

RCmerci commented Feb 17, 2023

what is the expected behavior if a page/journal contains local assets such as images?

For example, page A has content ![image](<localpath/of/this/image>), then the exported/copied content only contains the same content.
This feature mainly offers these functions:

  • replace block-ref, block-embed, and page-embed with their origin contents
  • some export options listed above
  • copy as a different format
  • and the content stays the same as the origin

@RCmerci RCmerci changed the title [wip] feat(editor/copy-paste): refactor copy/export in cljs feat(editor/copy-paste): refactor copy/export in cljs Feb 19, 2023
@giuseppedandrea
Copy link
Contributor

Hi @RCmerci! Does this refactor resolve the issue of the two trailing spaces added on each line on copy/export (#6351)?

@RCmerci
Copy link
Contributor Author

RCmerci commented Feb 19, 2023

@giuseppedandrea yes, this impl won't add 2 trailing spaces to each line

@tiensonqin tiensonqin force-pushed the refactor/export-copy branch 2 times, most recently from a9d85c8 to 9b6d7af Compare February 20, 2023 08:45
Copy link
Contributor

@tiensonqin tiensonqin left a comment

Choose a reason for hiding this comment

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

Works great! 👍

@tiensonqin tiensonqin merged commit 2a12ffc into master Feb 20, 2023
@tiensonqin tiensonqin deleted the refactor/export-copy branch February 20, 2023 15:09
@Bad3r Bad3r added this to the 0.8.18 milestone Feb 24, 2023
sallto pushed a commit to sallto/logseq that referenced this pull request Feb 25, 2023
Huge performance improvement for copy and export

Refactor copy/export in cljs instead of Mldoc
sallto pushed a commit to sallto/logseq that referenced this pull request Feb 25, 2023
Huge performance improvement for copy and export

Refactor copy/export in cljs instead of Mldoc
@stdword
Copy link
Contributor

stdword commented Mar 2, 2023

@RCmerci Currently properties of blocks can't be exported (it behaved like that before your PR). The place to add another one option?

The only way to copy all text blocks tree now is via selecting root block and using keyboard: ⌘C. There is no option to Copy in context menu. Only Copy as... but it trims all the properties

@RCmerci
Copy link
Contributor Author

RCmerci commented Mar 2, 2023

@stdword This PR mainly focuses on refactoring the existing code and does not add much new functionality.
I'll definitely add 'trim properties' option to 'copy as' feature later

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.

Two trailing spaces are added on each line
5 participants