Skip to content

Parse markdown and generate JSON file for exercises#94

Open
emmadal wants to merge 7 commits intomasterfrom
emmanueldalougou/nan-18-parse-markdown-and-generate-json-file
Open

Parse markdown and generate JSON file for exercises#94
emmadal wants to merge 7 commits intomasterfrom
emmanueldalougou/nan-18-parse-markdown-and-generate-json-file

Conversation

@emmadal
Copy link
Copy Markdown
Contributor

@emmadal emmadal commented Oct 19, 2021

Parse markdown and generate JSON file for exercises

@linear
Copy link
Copy Markdown

linear Bot commented Oct 19, 2021

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Oct 19, 2021

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0842a27
Status: ✅  Deploy successful!
Preview URL: https://fce05f6a.platform-nan-dev-8sl.pages.dev

View logs

Copy link
Copy Markdown
Member

@kigiri kigiri left a comment

Choose a reason for hiding this comment

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

ya encore d'autres trucs a review je pense mais tu peu deja avancer sur les points que j'ai mis en avant la

Comment thread dev/exo-parser.js Outdated
const isH2 = (node) => node.type === 'heading' && node.depth === 2
const isH3 = (node) => node.type === 'heading' && node.depth === 3
const text = (node) => node.type === 'paragraph'
const isList = (node) => node.type === 'list'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think text is a good name to check if something is a paragraph. I you want to keep it short call it isP and isList could be isLI so it's like HTML tags

just text give me the impression that it will return the text of the node

Comment thread dev/exo-parser.js Outdated
return content
}

const dirList = await readdir('js-introduction')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the name of the directory should be defined by the build.js file, not hard coded here

Comment thread dev/exo-parser.js Outdated
.flatMap((e) => e.children)
.flatMap((v) => v.children)
.map((k) => k.value)
content.notions = ul
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

fait plutot:

content.notions = children(node).map(getTrimValue).filter(Boolean)

Comment thread dev/exo-parser.js Outdated
} else if (mode === 'instructions') {
if (text(node)) {
let p = node.children.map((e) => e.value)
content.instructions = p.join('')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On veut garder la structure, juste stock toute la node, pas besoin de faire un cas special.
Comme ca on pourra render tout les element en (p)react

Comment thread dev/exo-parser.js
}
} else if (mode) {
// any other mode is stored in raw tree
content[mode].push(node)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

## Instructions pourra etre gerer ici

Comment thread dev/exo-parser.js Outdated
content[mode].push(node)
} else {
// before any mode is set, we are writing the description
content.description += textContent(node)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ca vaut ptet le coup de rajouter un \n quand on concatene ici:

content.description += `${textContent(node).trim()}\n`

Comment thread dev/exo-parser.js Outdated
await rename(
join(fileURLToPath(dirname(import.meta.url)), `../${file}`),
join(fileURLToPath(dirname(import.meta.url)), `../exoBundle/${file}`),
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ca serais bien d'avoir:

const rootDir = fileURLToPath(dirname(import.meta.url))

plutot que de le repeter a chaque fois

Comment thread dev/exo-parser.js Outdated
})

const rootFile = await (
await readdir('./')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

la t'utilise un chemin relatif, ton script va pas faire la meme chose en fonction du cwd, ou tu le run, c'est mieu de ce baser sur la position du fichier la avec fileURLToPath(dirname(import.meta.url)) comme tu l'a fait ailleur

Comment thread dev/exo-parser.js Outdated

export const generateExoFile = async () => {
dirList.map(async (filename) => {
const data = Object.fromEntries(entries)[`${filename}`]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[`${filename}`] -> [filename]

Comment thread dev/server.js Outdated
sendResponse({ body, options, res })
}).listen(PORT, () => console.log(`Dev server ready on ${process.env.DOMAIN}`))
}).listen(PORT, async () => {
await generateExoFile()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

il faudrais generer avant ca, et aussi s'assurer que les fichier sont accessible a fetch

Comment thread dev/exo-parser.js Outdated

const moveFile = async (file) =>
await rename(join(rootDir, file), join(rootDir, 'exoBundle', file))
await rename(join(rootDir, file), join(rootDir, 'exobundle', file))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

pas besoin d'async / await la:

const moveFile = (file) =>
  rename(join(rootDir, file), join(rootDir, 'exobundle', file))

Comment thread dev/exo-parser.js Outdated
await (await readdir(rootDir))
.filter((file) => file.includes('exercise.json'))
.map(moveFile)
return await Promise.all(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

pas besoin d'await quand tu return non plus

Comment thread dev/server.js Outdated

// generate bundle for js-introduction exercise
await getJsonExoFile()
await generateJSONExo()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

j'avais fais des commentaires a ce niveau, le generate doit etre fait pas build.js, non pas par le server.js

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

J'ai completement omis de l'enlever je le faisais pour des tests locaux

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.

2 participants