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

Ignore fenced code blocks #6

Merged
merged 3 commits into from
Mar 10, 2019

Conversation

dharmab
Copy link
Contributor

@dharmab dharmab commented Mar 9, 2019

Thanks for this useful tool! Here's a first pass at ignoring fenced code blocks as defined in the GFM spec. Fixes #4

I'd like to contribute tests for this, but the build_toc() function is currently somewhat difficult to test because it can only take input from files. I propose a change to the api to provide a build_toc_from_string() function which takes the markdown content as a string. Alternatively, I can add a bunch of test case markdown files to the repo. Let me know what approach you prefer.

For now, a brief manual test:

[dharmab@n7 md-toc]$ cat test.md
# Foo

[](TOC)

```python
# Bar
#
## Baz
```

````
# Bar
#
## Baz
`````

~~~
# Bar
#
## Baz
~~~
[dharmab@n7 md-toc]$ md_toc github test.md 
- [Foo](#foo)

When I ran make pep on master it changed a bunch of files, so I decided not to run make pep to keep the diff small. It would be helpful to include a requirements.txt file with pinned versions of tools like yapf and flake8. I know that flake8 changes its behavior quite a bit with each release.

@dharmab
Copy link
Contributor Author

dharmab commented Mar 9, 2019

More testing:

[dharmab@n7 md-toc]$ cat test.md 
# Foo1

[](TOC)

```python
# Bar
#
## Baz
```

# Foo2

````
# Bar
#
## Baz
`````

# Foo3

~~~
# Bar
#
## Baz
~~~

# Foo4
[dharmab@n7 md-toc]$ md_toc github test.md 
- [Foo1](#foo1)
- [Foo2](#foo2)
- [Foo3](#foo3)
- [Foo4](#foo4)

@frnmst
Copy link
Owner

frnmst commented Mar 9, 2019

Thanks for this useful tool! Here's a first pass at ignoring fenced code blocks as defined in the GFM spec. Fixes #4

Hello,

thanks!
Could you rebase the commit to the newly created frnmst:ignore-code-fences branch? I can't accept it in the master branch because of the PyPI and AUR packages. Then, once the dev branch is ready, we'll merge ignore-code-fences in dev.

At the moment I am working on the TOC list indentation problem because it was something I left behind for a long time. There is also the fact that GitLab changed markdown parser in the past year:

I'd like to contribute tests for this, but the build_toc() function is currently somewhat difficult to test because it can only take input from files. I propose a change to the api to provide a build_toc_from_string() function which takes the markdown content as a string. Alternatively, I can add a bunch of test case markdown files to the repo. Let me know what approach you prefer.

Yes, tests are mandatory for md_toc. The API changed (and it is still changing) in the dev branch. See:

Maybe it's possible to move the code fence detection part of your code to a new function, something like is_code_fence(parser, line, is_in_code_fence), so you can run tests easily. The only thing build_toc_line would do is to keep track of the state, just like it does for the headers. What do you think?

For now, a brief manual test:

[dharmab@n7 md-toc]$ cat test.md
# Foo

[](TOC)

```python
# Bar
#
## Baz
# Bar
#
## Baz
# Bar
#
## Baz

[dharmab@n7 md-toc]$ md_toc github test.md



When I ran make pep on master it changed a bunch of files, so I decided not to run make pep to keep the diff small. It would be helpful to include a requirements.txt file with pinned versions of tools like yapf and flake8. I know that flake8 changes its behavior quite a bit with each release.

I didn't know about this but I'll certainly look into it.
Thanks

@frnmst frnmst changed the base branch from master to ignore-code-fences March 10, 2019 09:39
@frnmst frnmst merged commit 0879578 into frnmst:ignore-code-fences Mar 10, 2019
@dharmab
Copy link
Contributor Author

dharmab commented Mar 13, 2019

👍 Splitting the code fence detection to two functions makes sense- one to detect the opening, and another to detect the closing. There is still state tracking that remains untested, but that's an easy way to both increase the test coverage and reduce the complexity of the main parser.

@frnmst
Copy link
Owner

frnmst commented Mar 14, 2019

Great, thanks!

@frnmst
Copy link
Owner

frnmst commented Mar 18, 2019

@frnmst
Copy link
Owner

frnmst commented Mar 18, 2019

Merged in the dev branch.

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