This repository has been archived by the owner on Sep 19, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 61
feat: change to markdown and map html to markdown #688
Draft
whilefoo
wants to merge
7
commits into
ubiquity:development
Choose a base branch
from
ubiquity-whilefoo:incentives-fix
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
09485ef
feat: change to markdown and map html to markdown
whilefoo b5446a8
fix: use old version of mdast libraries that can be used with require
whilefoo d00af2c
fix: markdown types
whilefoo 0b5e149
feat: html tags enum
whilefoo 9e88f65
Merge remote-tracking branch 'upstream/development' into incentives-fix
whilefoo ed023ba
feat: add config
whilefoo 8e6ddaa
Merge remote-tracking branch 'upstream/development' into incentives-fix
whilefoo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,42 +1,91 @@ | ||
| import * as parse5 from "parse5"; | ||
| import { MarkdownItem } from "../types"; | ||
|
|
||
| type Node = { | ||
| nodeName: string; | ||
| tagName?: string; | ||
| value?: string; | ||
| childNodes?: Node[]; | ||
| type: MarkdownItem; | ||
| value: string; | ||
| depth?: number; | ||
| children: Node[]; | ||
| }; | ||
|
|
||
| const traverse = (result: Record<string, string[]>, node: Node, itemsToExclude: string[]): Record<string, string[]> => { | ||
| if (itemsToExclude.includes(node.nodeName)) { | ||
| return result; | ||
| const traverse = (result: Record<MarkdownItem, string[]>, node: Node, itemsToExclude: string[]): Record<MarkdownItem, string[]> => { | ||
| if (!result[node.type]) { | ||
| result[node.type] = []; | ||
| } | ||
|
|
||
| if (!result[node.nodeName]) { | ||
| result[node.nodeName] = []; | ||
| if (node.type === MarkdownItem.Heading) { | ||
| node.type = `heading${node.depth}` as MarkdownItem; | ||
| } | ||
|
|
||
| result[node.nodeName].push(node.value?.trim() ?? ""); | ||
| result[node.type].push(node.value?.trim() ?? ""); | ||
|
|
||
| if (itemsToExclude.includes(node.type)) { | ||
| return result; | ||
| } | ||
|
|
||
| if (node.childNodes && node.childNodes.length > 0) { | ||
| node.childNodes.forEach((child) => traverse(result, child, itemsToExclude)); | ||
| if (node.children && node.children.length > 0) { | ||
| node.children.forEach((child) => traverse(result, child, itemsToExclude)); | ||
| } | ||
|
|
||
| return result; | ||
| }; | ||
|
|
||
| export const parseComments = (comments: string[], itemsToExclude: string[]): Record<string, string[]> => { | ||
| const result: Record<string, string[]> = {}; | ||
| export const parseComments = async (comments: string[], itemsToExclude: string[]): Promise<Record<MarkdownItem, string[]>> => { | ||
| // dynamic import of mdast | ||
| const { fromMarkdown } = await import("mdast-util-from-markdown"); | ||
| const { gfmFromMarkdown } = await import("mdast-util-gfm"); | ||
| const { gfm } = await import("micromark-extension-gfm"); | ||
|
|
||
| const result: Record<MarkdownItem, string[]> = { | ||
| [MarkdownItem.Text]: [], | ||
| [MarkdownItem.Paragraph]: [], | ||
| [MarkdownItem.Heading]: [], | ||
| [MarkdownItem.Heading1]: [], | ||
| [MarkdownItem.Heading2]: [], | ||
| [MarkdownItem.Heading3]: [], | ||
| [MarkdownItem.Heading4]: [], | ||
| [MarkdownItem.Heading5]: [], | ||
| [MarkdownItem.Heading6]: [], | ||
| [MarkdownItem.ListItem]: [], | ||
| [MarkdownItem.List]: [], | ||
| [MarkdownItem.Link]: [], | ||
| [MarkdownItem.Image]: [], | ||
| [MarkdownItem.BlockQuote]: [], | ||
| [MarkdownItem.Code]: [], | ||
| [MarkdownItem.Emphasis]: [], | ||
| [MarkdownItem.Strong]: [], | ||
| [MarkdownItem.Delete]: [], | ||
| [MarkdownItem.HTML]: [], | ||
| [MarkdownItem.InlineCode]: [], | ||
| [MarkdownItem.LinkReference]: [], | ||
| [MarkdownItem.ImageReference]: [], | ||
| [MarkdownItem.FootnoteReference]: [], | ||
| [MarkdownItem.FootnoteDefinition]: [], | ||
| [MarkdownItem.Table]: [], | ||
| [MarkdownItem.TableCell]: [], | ||
| [MarkdownItem.TableRow]: [], | ||
| [MarkdownItem.ThematicBreak]: [], | ||
| [MarkdownItem.Break]: [], | ||
| [MarkdownItem.Root]: [], | ||
| [MarkdownItem.Definition]: [], | ||
| }; | ||
|
|
||
| for (const comment of comments) { | ||
| const fragment = parse5.parseFragment(comment); | ||
| traverse(result, fragment as Node, itemsToExclude); | ||
| const tree = fromMarkdown(comment, { | ||
| extensions: [gfm()], | ||
| mdastExtensions: [gfmFromMarkdown()], | ||
| }); | ||
| console.log(`Comment Mdast Tree: ${JSON.stringify(tree, null, 2)}`); | ||
| traverse(result, tree as Node, itemsToExclude); | ||
| } | ||
|
|
||
| console.log(`Comment Parsed: ${JSON.stringify(result, null, 2)}`); | ||
|
|
||
| // remove empty values | ||
| if (result["#text"]) { | ||
| result["#text"] = result["#text"].filter((str) => str.length > 0); | ||
| if (result[MarkdownItem.Text]) { | ||
| result[MarkdownItem.Text] = result[MarkdownItem.Text].filter((str) => str.length > 0); | ||
| } | ||
|
|
||
| console.log(`Comment Parsed Cleaned: ${JSON.stringify(result, null, 2)}`); | ||
|
|
||
| return result; | ||
| }; | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,63 @@ | ||
| export const MarkdownItem = { | ||
| Paragraph: "paragraph", | ||
| List: "list", | ||
| Link: "link", | ||
| Text: "text", | ||
| Code: "code", | ||
| Image: "image", | ||
| BlockQuote: "blockquote", | ||
| } as const; | ||
| export type MarkdownItem = (typeof MarkdownItem)[keyof typeof MarkdownItem]; | ||
| // All types supported by standard Markdown and GFM | ||
| export enum MarkdownItem { | ||
| // Standard Markdown | ||
| BlockQuote = "blockquote", | ||
| Break = "break", | ||
| Code = "code", | ||
| Definition = "definition", | ||
| Emphasis = "emphasis", | ||
| Heading = "heading", | ||
| Heading1 = "heading1", | ||
| Heading2 = "heading2", | ||
| Heading3 = "heading3", | ||
| Heading4 = "heading4", | ||
| Heading5 = "heading5", | ||
| Heading6 = "heading6", | ||
| HTML = "html", | ||
| Image = "image", | ||
| ImageReference = "imageReference", | ||
| InlineCode = "inlineCode", | ||
| Link = "link", | ||
| LinkReference = "linkReference", | ||
| List = "list", | ||
| ListItem = "listItem", | ||
| Paragraph = "paragraph", | ||
| Root = "root", | ||
| Strong = "strong", | ||
| Text = "text", | ||
| ThematicBreak = "thematicBreak", | ||
| // GFM | ||
| Delete = "delete", | ||
| FootnoteDefinition = "footnoteDefinition", | ||
| FootnoteReference = "footnoteReference", | ||
| Table = "table", | ||
| TableCell = "tableCell", | ||
| TableRow = "tableRow", | ||
| } | ||
| export const MarkdownItems = [ | ||
| MarkdownItem.BlockQuote, | ||
| MarkdownItem.Break, | ||
| MarkdownItem.Code, | ||
| MarkdownItem.Definition, | ||
| MarkdownItem.Emphasis, | ||
| MarkdownItem.Heading, | ||
| MarkdownItem.HTML, | ||
| MarkdownItem.Image, | ||
| MarkdownItem.ImageReference, | ||
| MarkdownItem.InlineCode, | ||
| MarkdownItem.Link, | ||
| MarkdownItem.LinkReference, | ||
| MarkdownItem.List, | ||
| MarkdownItem.ListItem, | ||
| MarkdownItem.Paragraph, | ||
| MarkdownItem.Root, | ||
| MarkdownItem.Strong, | ||
| MarkdownItem.Text, | ||
| MarkdownItem.ThematicBreak, | ||
| MarkdownItem.Delete, | ||
| MarkdownItem.FootnoteDefinition, | ||
| MarkdownItem.FootnoteReference, | ||
| MarkdownItem.Table, | ||
| MarkdownItem.TableCell, | ||
| MarkdownItem.TableRow, | ||
| ] as const; |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried importing normally at the top but I got the error:
After that I changed to dynamic import in the function but the error stays the same. I checked the compiled javascript and it's still
require()- it seems everyimportis compiled torequire.@rndquu any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is a related issue #378
And this is a related PR #377 (which had been reverted because we got rid of
mdastpackages)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems #377 was supposed to fix the issue but it doesn't - instead of throwing the error at startup, it throws the error when the function runs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it throws the same error both when modules are imported at the top of the
comment.tsfile and in theparseComments()function, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried changing Typescript settings to transpile to ESM and added
"type": "module"topackage.json, but I get a different error because Probot is usingrequire()internallynode_modules/probot/lib/helpers/resolve-app-function.jsis usingrequire()There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we could downgrade all 3 packages (
mdast-util-from-markdown,mdast-util-gfm,micromark-extension-gfm) to their "pre ESM" versionsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried downgrading and I have an issue with
mdast-util-gfmbecause that version didn't have any types so typescript is complaining. I will see if there's anything I can doThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/ubiquity/ubiquibot/blob/incentive-based-on-comment/src/helpers/comment.ts#L23
Please take a look at the
incentive-based-on-commentbranch @whilefoo. It has been done withmdastlibrary.The blocker on that branch was the function timeout: 10s as of now.
I hope you can get reusable/helpful methods from that branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is exactly how I've done it but it's not working, you can check the whole conversation for context