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: serialize AST node back to wikitext string #8258

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

linonetwo
Copy link
Contributor

@linonetwo linonetwo commented Jun 12, 2024

closes #8255

Currently just a demo, I will gradually move code from https://github.com/tiddly-gittly/wikiast/tree/master/packages/wikiast-util-to-wikitext to here.

Try it with tiddler (Well, if official website have share plugin installed, I can put a link here):

AAA test

---

\```js
var match = reEnd.exec(this.parser.source)
\```

end

and run this in console:

$tw.utils.serializeParseTree($tw.wiki.parseTiddler('New Tiddler').tree) 

Copy link

vercel bot commented Jun 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
tiddlywiki5 ✅ Ready (Inspect) Visit Preview Jun 12, 2024 5:39pm

@linonetwo linonetwo changed the title Feat/to string feat: serialize AST node back to wikitext string Jun 12, 2024
@pmario
Copy link
Contributor

pmario commented Jun 13, 2024

The problem here is, that the output of you test does not produce the same rendered HTML output as the original source. See the screenshot.

image

@linonetwo
Copy link
Contributor Author

linonetwo commented Jun 13, 2024

First line is wrapped by a p element, but I haven't find the rule that produce the p element. And there was no p html in the source, so I may need to add some attribute to ast to show this p element is "fake".

Anyway, this is just a demo for API, do you like the API name and their usage? The further task is just adding exports.serailize = xxx to each rule.

@pmario
Copy link
Contributor

pmario commented Jun 13, 2024

First line is wrapped by a p element, but I haven't find the rule that produce the p element. And there was no p html in the source, so I may need to add some attribute to ast to show this p element is "fake".

IMO the current parser has a "systemic" problem with redundant P-tags, that we probably may need to solve first.

Working around this problem with "fake" elements will just increase your code complexity, instead of solving the underlaying problem. It will also create an other dependency, that will make solving the problem, in the core, in backwards compatible way even harder as it is now.

};

exports.getParser = function(type,options) {
options = $tw.utils.extend({},options);
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO your code will throw an RSOD if options is undefined
So options = options || {} should be used instead. We use this pattern all over the places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copy $tw.utils.extend({},options) from somewhere nearby, I thought this will work.

@linonetwo
Copy link
Contributor Author

redundant P-tags

Can't remove then, many of my plugin style sheet is already depending on this structure. And p > text for block is following HTML5 semantic.

Another option is handle it like text node, regard it as a special case.

@pmario
Copy link
Contributor

pmario commented Jun 13, 2024

And p > text for block is following HTML5 semantic.

That's true, but we do have a lot of p > div which should not happen at all.

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.

[IDEA] Serailize/toString of AST node
2 participants