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

Changing logic for converting markdown to json and back #4

Closed
wants to merge 1 commit into from
Closed

Changing logic for converting markdown to json and back #4

wants to merge 1 commit into from

Conversation

andrewshell
Copy link

Here's what I felt should be changed:

  1. In stringify there was a typo where you referenced opmlToJs and the variable was all lowercase.
  2. At the top of the markdown file, there can be page-level attributes that should be stored as head attributes
  3. Attributes under a headline have tabs for the indenting, but then two spaces so they align with the text of the headline.
  • You were not removing the spaces so an attribute like collapsed:: true was turned into
  • OPML of _ collapsed="true" which is not valid

I've created a small project that takes a sample LogSeq markdown file and converts it to JSON, OPML, and a new markdown file and compares it with the original. https://github.com/andrewshell/test-opml-md

@scripting
Copy link
Owner

@andrewshell --

I started a thread for discussion of interop between Drummer and LogSeq et al. Whatever changes you feel are necessary must be discussed there, not here.

scripting/drummerRFC#4 (comment)

@scripting scripting closed this Jan 5, 2022
@scripting
Copy link
Owner

@andrewshell -- I looked into the first item in the list, and it's unclear to me how that code ever worked, but it does work, I use that routine all the time. Very weird.

Anyway I hope you do join that other thread. We're going to get this right, it's going to be iterative, and slow, and done with enough consideration.

@scripting
Copy link
Owner

OK -- I did break it, it was a stupid error, of course, but it's now fixed. That's for catching that. ;-)

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.

None yet

2 participants