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

Fix line number / cell index reporting #71

Closed
chrisjsewell opened this issue Mar 14, 2020 · 2 comments · Fixed by #107
Closed

Fix line number / cell index reporting #71

chrisjsewell opened this issue Mar 14, 2020 · 2 comments · Fixed by #107
Labels
bug Something isn't working enhancement New feature or request

Comments

@chrisjsewell
Copy link
Member

So that sphinx will report errors/warnings referring to correct place in the notebook

@chrisjsewell chrisjsewell added enhancement New feature or request bug Something isn't working labels Mar 14, 2020
@choldgraf
Copy link
Member

Could we pass in the cell number into the state machine somehow? I feel like cell numbers are more helpful to report here than line numbers

@chrisjsewell
Copy link
Member Author

chrisjsewell commented Mar 15, 2020

Well ideally you'd report both. Its just thinking of a way to do this that ideally still keeps myst-parser (where some of this logic would need to be added) agnostic to notebooks.

Note the cell index is now stored in the markdown AST, which was the first step: https://github.com/ExecutableBookProject/MyST-NB/blob/4c3f36b884086878dc0af5dfcfd735b98ebbbacf/myst_nb/parser.py#L94
its just then how to get this into the docutils AST and reported by the docutils reporter.

chrisjsewell added a commit that referenced this issue Mar 29, 2020
Report line number as <cell index>*10000 + <line number>. This is a simple solution to addresses #71, that doesn't require any complex overrides of the sphinx reporting machinery.
@chrisjsewell chrisjsewell linked a pull request Mar 29, 2020 that will close this issue
chrisjsewell added a commit that referenced this issue Apr 1, 2020
This commit move to markdown-it-py markdown parser implementation, concurrently with myst-parser. Additionally:

- Add notebook render tests
- Add simple solution for reporting correct cell index/line number:
  Report line number as <cell index>*10000 + <line number>. This is a simple solution to addresses #71, that doesn't require any complex overrides of the sphinx reporting machinery.
- Make tests use the actual sphinx Application
- Re-write validation of which docs to execute/cache:
  Rather than having a global variable, we save the exclude paths in the sphinx env and use a seperate function `is_valid_exec_file`. Also added tests
phaustin pushed a commit to phaustin/MyST-NB that referenced this issue Apr 1, 2020
…s#107)

This commit move to markdown-it-py markdown parser implementation, concurrently with myst-parser. Additionally:

- Add notebook render tests
- Add simple solution for reporting correct cell index/line number:
  Report line number as <cell index>*10000 + <line number>. This is a simple solution to addresses executablebooks#71, that doesn't require any complex overrides of the sphinx reporting machinery.
- Make tests use the actual sphinx Application
- Re-write validation of which docs to execute/cache:
  Rather than having a global variable, we save the exclude paths in the sphinx env and use a seperate function `is_valid_exec_file`. Also added tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants