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

doc warning fix #385

Closed
wants to merge 1 commit into from
Closed

doc warning fix #385

wants to merge 1 commit into from

Conversation

mindw
Copy link

@mindw mindw commented Feb 21, 2015

No description provided.

@@ -237,7 +237,7 @@ This is the current list of error and warning codes:
| E116 | unexpected indentation (comment) |
+----------+----------------------------------------------------------------------+
+----------+----------------------------------------------------------------------+
| E121 (*^)| continuation line under-indented for hanging indent |
| E121(\*^)| continuation line under-indented for hanging indent |
Copy link
Member

Choose a reason for hiding this comment

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

I think we still want it to be:

| E121 (\*^) |

Thanks for noticing that the * wasn't appearing properly though.

Copy link
Author

Choose a reason for hiding this comment

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

This will require to re-align the entire table.

Copy link
Member

Choose a reason for hiding this comment

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

All of the other errors have spaces between the ( and the error code. Re-aligning the table is quite simple to do if you use vim or sublime text

@mindw
Copy link
Author

mindw commented Feb 22, 2015

Well, using sublime + Table Editor did the trick.

| W604 | backticks are deprecated, use 'repr()' |
+----------+----------------------------------------------------------------------+
+------------+----------------------------------------------------------------------+
| code | sample message |
Copy link
Member

Choose a reason for hiding this comment

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

Why did you center "code" and "sample message"?

@mindw
Copy link
Author

mindw commented Feb 22, 2015

sublimetext 3 + Table Editor = center "code" and "sample message".

Does it matter?

@sigmavirus24
Copy link
Member

Does it matter?

I'm not sure it's a blocker, but it's confusing to see as part of this change. It's also unclear if docutils/sphinx will render this differently as a result.

It's also good etiquette to not make entirely unnecessary changes in a pull request, so there's that.

@mindw
Copy link
Author

mindw commented Feb 28, 2015

Let me rephrase:

  1. No, I did not center the headings intentionally.
  2. I could not notice and change in the output:
    Original - http://rst.ninjs.org/?n=67d2461d8e0e40a9eb3bb83a13593cc1&theme=basic
    Patch - http://rst.ninjs.org/?n=06979870b8cf9567972c18e8c9c0694d&theme=basic
  3. Given the above do you want this change reverted? kept?

@IanLee1521
Copy link
Member

This appears to have been fixed in 31c0215 and 37648d3, thanks for the report @mindw !

@IanLee1521 IanLee1521 closed this Jun 6, 2016
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

3 participants