-
-
Notifications
You must be signed in to change notification settings - Fork 2k
graphviz syntax error generated by Sphinx 1.2b1 #936
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
Conversation
|
It appears to be putting unescaped quotes within strings. |
|
@mdboom - did you get it to build sucessfully with the fix you posted? I get the error I've included below when I use the latest sphinx tip with your addition: |
|
@eteq: With the patch here, it completes for me. What you've posted seems to be an instance of the same error, so I'm not sure why it's not working for you. Is your patched Sphinx the same one being used by the |
|
@mdboom: I think I figured it out - the fix you have in the sphinx issue does is |
|
Oh! I just discovered this myself. Didn't realize this was something we were already aware of but this was the first hit on Google when I finally went to search for the issue. I'm having this problem on Windows myself. |
|
Except that I'm getting this on Windows even with Sphinx 1.1.3. It seems to be a slightly different syntax error--it doesn't like the |
|
@iguananaut - that's weird... I wonder if it's a different problem, then? And do you think it's a sphinx problem, then, or our problem? |
|
Oh I'm almost certain it's a Sphinx problem, and probably Windows specific. Like most open source projects I doubt it gets much Windows testing ;) |
|
@mdboom - can this be closed now that Sphinx includes the bug fix from your PR? |
|
I think we should update our minimum required Sphinx version, though. |
|
Attached a commit that updates the minimum Sphinx version. Since docs building is primarily a problem for developers, I don't see this as introducing a significant burden. |
|
@mdboom - sounds good, feel free to merge! |
graphviz syntax error generated by Sphinx 1.2b1
|
@mdboom @astrofrog - I don't think we actually wanted to do this. The latest version is not yet released, and this is causing all kinds of travis-related build problems you may or may not have noticed. I also don't think we want to update the minimum version, anyway, as that's the minimum version required to build the docs (e.g., we have features that require 1.1, but none that require 1.2). Also, remember that this conf.py propogates to affiliated packages, so now all their docs might have the same problems until sphinx 1.2 is released. So I think we should revert back to 1.1 for now. It works fine if you use sphinx 1.1.3 and an appropriate graphviz. |
|
Oh, wait, ignore this - I just saw #1478 |
|
We can go two ways -- we can require dev sphinx or require users to downgrade their graphviz. I think the former is much easier to do. |
…x-version graphviz syntax error generated by Sphinx 1.2b1
After fixing #935, this other bug was revealed beneath. It's probably "upstream fix required" but I'm adding it here so we can track our compatibility with Sphinx 1.2.