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

rst2html: Extend the field_name_limit to 50 #306

Merged

Conversation

leandro-lucarella-sociomantic
Copy link
Contributor

Field lists are rendered weirdly in reST if the field name is longer that 14 characters. In this case the table cell where the field name is gets a colspan=2.

For example this:

:123456789012345: too long, colspan=2
:12345678901234: OK

Gets translated to:

<table class="docinfo" frame="void" rules="none">
<col class="docinfo-name" />
<col class="docinfo-content" />
<tbody valign="top">
<tr class="field"><th class="docinfo-name"
colspan="2">123456789012345:</th></tr>
<tr class="field"><td>&nbsp;</td><td class="field-body">too long,
colspan=2</td>
</tr>
<tr class="field"><th class="docinfo-name">12345678901234:</th><td
class="field-body">OK</td>
</tr>
</tbody>
</table>

Increasing this default value of 14 to something more reasonable, like 50, seems sensible. Even when this is just another arbitrary value, since GitHub have a fixed total width, it seems sensible to not make it
much more longer, since if the field name is too long it will look weird anyway.

@leandro-lucarella-sociomantic
Copy link
Contributor Author

BTW, this is how it looks without this PR:

field-list

And this is how it should look like with this PR (I didn't check because I don't know how to test it):

field-list-ok

@bkeepers
Copy link
Contributor

Thanks for the pull request! I think that makes sense.

Do you mind adding the example to test/markups/README.rst and updating test/markups/README.rst.html to include the expected rendered output so we can make sure we don't break this in the future?

/cc @gjtorikian @vmg

Field lists are rendered weirdly in reST if the field name is longer
that 14 characters. In this case the table cell where the field name is
gets a colspan=2.

For example this:

```rest
:123456789012345: too long, colspan=2
:12345678901234: OK
```

Gets translated to:

```html
<table class="docinfo" frame="void" rules="none">
<col class="docinfo-name" />
<col class="docinfo-content" />
<tbody valign="top">
<tr class="field"><th class="docinfo-name"
colspan="2">123456789012345:</th></tr>
<tr class="field"><td>&nbsp;</td><td class="field-body">too long,
colspan=2</td>
</tr>
<tr class="field"><th class="docinfo-name">12345678901234:</th><td
class="field-body">OK</td>
</tr>
</tbody>
</table>
```

Increasing this default value of 14 to something more reasonable, like
50, seems sensible. Even when this is just another arbitrary value,
since GitHub have a fixed total width, it seems sensible to not make it
much more longer, since if the field name is too long it will look weird
anyway.
@leandro-lucarella-sociomantic
Copy link
Contributor Author

On Tue, May 27, 2014 at 12:40:36PM -0700, Brandon Keepers wrote:

Thanks for the pull request! I think that makes sense.

Do you mind adding the example to test/markups/README.rst and
updating test/markups/README.rst.html to include the expected
rendered output so we can make sure we don't break this in the future?

Updated, let me know if this is what you had in mind.

@leandro-lucarella-sociomantic
Copy link
Contributor Author

Anything else I could do to convince you to merge this? :)

@gjtorikian
Copy link
Contributor

LGTM.

@leandro-lucarella-sociomantic
Copy link
Contributor Author

Merge? Maybe? Please? :)

@bkeepers
Copy link
Contributor

bkeepers commented Jul 3, 2014

I'm on vacation right now, but will merge it next week if it hasn't been done before then.

@bkeepers bkeepers self-assigned this Jul 3, 2014
bkeepers added a commit that referenced this pull request Sep 11, 2014
…-limit

rst2html: Extend the field_name_limit to 50
@bkeepers bkeepers merged commit 2a14dc2 into github:master Sep 11, 2014
@bkeepers
Copy link
Contributor

Thanks, sorry for the delay!

@leandro-lucarella-sociomantic
Copy link
Contributor Author

💃

@leandro-lucarella-sociomantic leandro-lucarella-sociomantic deleted the field-name-limit branch September 12, 2014 15:14
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.

3 participants