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

Open
wants to merge 55 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
Member

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
Member

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
Member

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
Member

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.

@linonetwo
Copy link
Contributor Author

linonetwo commented Jul 29, 2024

Handle the block rule by adding a rule: 'parseBlock' to WikiParser.prototype.parseBlock.

I've added some test for it.

@linonetwo
Copy link
Contributor Author

linonetwo commented Aug 7, 2024

The LLM prompt

update this to add exports.serialize = function(tree, serialize) { that accept node and a serialize that accept array of node or a node and return a string ,Use ES5 only in the serialize , and only output this method. No explain but add comments for internal variable's possible example value and intention.Don't add comment that is already explained by variable name. And add comment on top of the line. No space after comma. And provide a test in another codeblock, use example in the top comment from rule code. Not like serialize code, No space and extra comment in test code

@linonetwo
Copy link
Contributor Author

All tests passed, opened for review.

@linonetwo linonetwo marked this pull request as ready for review August 9, 2024 18:37
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