Fix attribute errors, address code-blocks #107
Conversation
|
Ah, just noticed that in fixing this I've exposed a bug in |
Well, I have a fix for that, but I can't get the tests to actually pick up the error. I may need a little help (or at least someone to verify I'm not crazy). |
|
Hey, thanks for filing this PR. |
This was a neat idea, but unfortunately upstream sphinx keeps multiple references to itself and internal structures, making a web of variables that do not leave the execution context once loaded. This causes a litany of parsing errors that have only gotten worse as sphinx progresses. For now, load one instance of sphinx to interact with if sphinx is available on the system. Signed-off-by: Randolph Sapp <rs@ti.com>
Try and generate a line number for a code-block in the event one is not provided. Right now this only seems to occur with indented code-blocks, which means we should be able to gleam some information from the parent node if it exists. Signed-off-by: Randolph Sapp <rs@ti.com>
Update the code-block example to also have a valid include file. This should generate an index error in _get_code_block_directive_line currently. Signed-off-by: Randolph Sapp <rs@ti.com>
59a61b9 to
a91b040
Compare
Handle code-blocks from files being included by fetching and operating on the source of the included file as needed. Add a function to iterate through parent nodes until we get a source since there is a chance the code-block could be nested several layers below other directives. Signed-off-by: Randolph Sapp <rs@ti.com>
a91b040 to
c010310
Compare
| @@ -397,21 +394,16 @@ def test_code_block_without_language_is_works_with_sphinx_pre310( | |||
| """ | |||
| ignores = types.construct_ignore_dict() | |||
| # fmt: off | |||
There was a problem hiding this comment.
All 6 remaining fmt comments can be removed. They were there to fixate the empty line between the now removed context manager and the variable assignement.
|
Hey, thanks for reviewing this, we've been running it in production for a little bit over here: https://github.com/TexasInstruments/processor-sdk-doc/ I've noticed one potential regression around code-blocks: the sphinx version of the code-block directive doesn't like the additional parameters (like I'll take a swing at it later today if it is a regression. If not I'll put it on a different PR since this one is creeping. |
|
From the top of my head I cannot recall any issues so far, but I am not working much with rst files since like 2 years or so and for code blocks I think I only used the :language: parameter. |
|
Alright, it is unfortunately a regression. Seems that the code-block directive is just being aliased to the default code directive and doc-utils is being passed the sphinx specific parameters. I'll take a swing at that before addressing the above. |
|
Ah, it's an issue with reloading docutils and not reloading sphinx at the same time. Unfortunately reloading sphinx did not work well the last time I tested that, even with a custom recursive module reload function. |
|
Ah, well, upstream sphinx was already addressing most of the state issues they had during testing using this: This PR can be dramatically simplified. Let me fix that. |
|
New issue. There's a problem with multiprocessing and attempting to interact with global caches. Technically this was always an issue. Now it's just glaringly obvious. |
|
Hi, how is the state of this PR? Things still need changes? |
|
Yeah, sorry about that. Got sidetracked in some other issues. I still need to take a good look at this. I've thought about (ab)using subprocesses to promote proper isolation between threads and sphinx instances. Even using the sphinx test helper functions, there seems to be some state either kept in it or docutils that negatively affects parsing of future documents. Messy stuff. |
Take a stab at addressing the attribute errors in sphinx now by eliminating the dynamic loader. A single instance of sphinx will be created at launch if sphinx is present on the system.
Also, address the warning about being unable to determine the line of a code-block by generating one from the information available in the parent node.
Let me know if there's anything else that needs addressing here. This is the first project I've played with that uses tox.
Resolves: #3
toxtest suite locally, so the CI pipelines should be green.