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

remove rst table border #220

Merged
merged 1 commit into from Sep 30, 2013

Conversation

Projects
None yet
2 participants
@zonyitoo
Copy link
Contributor

zonyitoo commented Sep 28, 2013

Description

Python's docutils generates a table with border=1 by default (See # 1).

This looks out of place with gh's look and feel, see

Before:
_111

After:
_112

Fixes the hard-coded border=1 in the html4css1 parser.

Link # 1: http://sourceforge.net/p/docutils/code/7661/tree/trunk/docutils/docutils/writers/html4css1/__init__.py#l1556

Scope: github rest2html parser. docutils.writer.html4css1

@gjtorikian

This comment has been minimized.

Copy link
Contributor

gjtorikian commented Sep 30, 2013

I recognize this is a problem, and will set out to fix it, but the suggestion made here is not a proper one.

Basically, RSTs are messed up because the DOM structure is totally wrong. For example, here's a table in a Markdown README. The DOM looks like this:

<article class="markdown-body entry-content" itemprop="mainContentOfPage">
  . . .
</article>

Meanwhile, for an RST README, the DOM looks like this:

<article class="markdown-body entry-content" itemprop="mainContentOfPage">
  <div>
    <div>
      . . .
    </div>
  </div>
</article>

While setting border=0 is a great quick fix, I don't know what other elements are being rendered poorly because of the div-in-div container. I'd rather eliminate that entirely, instead.

@gjtorikian

This comment has been minimized.

Copy link
Contributor

gjtorikian commented Sep 30, 2013

Forget everything I said, I was completely wrong. This looks good! Tests added in #222. Thanks!

gjtorikian added a commit that referenced this pull request Sep 30, 2013

Merge pull request #220 from zonyitoo/master
remove rst table border

@gjtorikian gjtorikian merged commit 75b6b02 into github:master Sep 30, 2013

1 check passed

default The Travis CI build passed
Details
@gjtorikian

This comment has been minimized.

Copy link
Contributor

gjtorikian commented Sep 30, 2013

/cc @tnm for one more fix to add to a gem release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment