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: improve body parsing logic #72

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Ansuel
Copy link

@Ansuel Ansuel commented Apr 26, 2024

Current body parsing logic with trim() + split("###") is too fragile and
pose problems with some body that contains case with ### in the middle
of the line or case with codeblock ``` section.

These case will cause the script to parse these as separate section and
produce wrong outputs and in some case even prints error assuming things
are checkbox and errors out on the concat function.

To make the parsing logic more solid, implement a dedicated function and
parse with this logic:

  • We split the body for "\n"
  • We ignore codeblock ``` section
  • We check "###" only at the start of the line
  • We check for "### " (with the space included) as that is the correct
    section Github issue template expects.

With the following change case like:

root@OpenWrt:~# cat /boot/config.txt

Are correctly parsed as all part of a single section instead of being
wrongly treated as different empty sections.

Signed-off-by: Christian Marangi ansuelsmth@gmail.com

Current body parsing logic with trim() + split("###") is too fragile and
pose problems with some body that contains case with ### in the middle
of the line or case with codeblock ``` section.

These case will cause the script to parse these as separate section and
produce wrong outputs and in some case even prints error assuming things
are checkbox and errors out on the concat function.

To make the parsing logic more solid, implement a dedicated function and
parse with this logic:
- We split the body for "\n"
- We ignore codeblock ``` section
- We check "###" only at the start of the line
- We check for "### " (with the space included) as that is the correct
  section Github issue template expects.

With the following change case like:
```
root@OpenWrt:~# cat /boot/config.txt
```

Are correctly parsed as all part of a single section instead of being
wrongly treated as different empty sections.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Add additional test for section with #### to prevent and catch in
future code change that would produce wrong parsing.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
@Ansuel
Copy link
Author

Ansuel commented Apr 26, 2024

Discovered by this openwrt/openwrt#15265

Made further research and found the root of the problem!

Add test for codeblock ``` ignore to prevent and catch in future code
change that would produce wrong parsing.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
@Ansuel Ansuel force-pushed the (metal-gear-)solid-parsing branch from 20e3584 to 7b837e5 Compare April 26, 2024 17:26

body.split("\n").reduce((str, line, idx, arr) => {
// ``` Section must be ignored and not parsed as new section
if (line == "```")
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also account for instances when the context of the code block is specified. For example:

```md

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