Fix Solarized line number colors #1477
Conversation
…zed-line-number-colors
pfertyk
changed the title
|
@Anteru are there any other fixes you would like me to include? |
|
Nope, looks good, I'll merge as soon as I have some more time :) |
Anteru
added
the
changelog-update
Anteru
removed
the
changelog-update
|
|
||
| @property | ||
| def _pre_style(self): | ||
| return 'line-height: 125%; margin: 0;' |
There was a problem hiding this comment.
Just curious - in which way margin: 0 makes it look better? In my case, it looks less pretty because there is no margin after the code. This looks more obvious when you write some explanation on the top and code after that few times, for example:
There was a problem hiding this comment.
Another example:
With margin: 0
Without margin: 0
I trying to override the CSS now and I was curious why this change was made.
There was a problem hiding this comment.
You're seeing (I guess) because your theme doesn't have margin/padding on the paragraph either. I'd try to override the style if you care about specific padding/margin. That said, the margin: 0 was not intentional (I think) so I'll revert that for 2.7.3.
There was a problem hiding this comment.
Hi! I'm sorry if I'm a bit too late, but I'm pretty I've introduced the margin: 0 rule for a reason. Since I've worked on this one some time ago, I'm not sure what exactly was it, but I'd guess it had something to do with making the table and non-table styles look more similar. An alternative might be that it had something to do with highlighting lines/special lines. There was a lot of different combinations of styles and highlight rules etc. and I believe margin: 0 was introduced to make everything work better in regard to some of these combinations.
Having said that, if it actually looks better without margin: 0 and it doesn't break anything else, I'm totally OK with removing this rule :)
There was a problem hiding this comment.
I can't find the case where it would matter, and it does break various themes -- if you can point me to a problematic case then we can revisit alternative solutions. Ideally theme authors should have a way to override Pygments CSS rules, but for now this seems to be causing too much fallout :(
There was a problem hiding this comment.
I tried to add all problematic cases to the test suite :P But while working on this a lot of dependencies came up and it's possible I've changed one thing too many ;) If you could just add a proper test with an explanation/justification, I would be very grateful. Right now, unfortunately, I'm not able to spend any time fixing this or checking why exactly was this change made. Reverting it seems like a good solution, as long as it's documented ;)
There was a problem hiding this comment.
Fair enough. I ran a bunch of manual tests and also spot-checked the test cases, and couldn't find a problematic case. Was hoping you'd remember something particularly problematic, but given the feedback we got, it seems that any rule changing the margin will cause more harm than good :(
|
@Anteru, please allow me a quick question here: when is expected release date of 2.7.3? |
|
Today. |
|
Thanks! |
Fixes #1356.
Changes:
get_style_defs(and removed fromCSSFILE_TEMPLATE)noclassesoption was updatedStyleclass received new attributes for: