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

Line numbers in syntax highlighting #157

Open
quicknir opened this issue Feb 13, 2016 · 14 comments
Open

Line numbers in syntax highlighting #157

quicknir opened this issue Feb 13, 2016 · 14 comments
Projects

Comments

@quicknir
Copy link

I've made very minimal changes from the base lanyon repo, and none to the CSS. I have the following code:

{% highlight cpp linenos %}
class A {
  void func(double y) {
    return;
  }
}
{% endhighlight %}

The problem is it looks like this:

screenshot from 2016-02-13 16-18-02

I have googled around for hours, trying bits and snippets of other people's CSS to try to fix this line number issue. Unfortunately, nothing seems to work. Because of how CSS works, I thought maybe some other piece of CSS on Lanyon could be causing this issue. Does anyone have nice looking line numbers on Lanyon, on a github-pages site that is build server-side (i.e. not locally)? If so, can they share?

IMHO this should be merged to Lanyon in some form, as wanting line numbers in code is a pretty common thing.

@bernardosulzbach
Copy link

Does anyone have nice looking line numbers on Lanyon, on a github-pages site that is build server-side (i.e. not locally)?

Why so specific? Do you get better line numbers when building locally?

Also try to give your issues a descriptive name, it is very hard to guess what is the problem you are having without reading the whole thing. See if Shift + Print Screen does not allow you to print just a section of the window on Ubuntu.

@quicknir
Copy link
Author

If you're going to build locally, then there are custom plugins and so on for Jekyll that are not available on github, and you have complete control of the build environment (which version of Jekyll, you can still use Pygments whereas github only supports Rouge, etc). So someone may have a solution that works for them building locally but will not work for me building on github (which is what I'd prefer).

I can change the title if you want, I think it's reasonably specific already to be frank. I could add "have excessive surrounding whitespace and look strange" to the end, but this seems a little long for an issue title, no? If you can suggest a better title, by all means.

I can crop the picture if you'd really like, but I can easily see exactly what's going on even with the existing picture, so I don't see what the issue is. Are you unable to see it?

@bernardosulzbach
Copy link

The title is as specific as "pig" is. It does not give anyone a clue about what the problem is. Line numbers in syntax highlighting, trains in railroads, Jeremy on unicorns... See why it is simply bad?

"have excessive surrounding whitespace and look strange"

This is the first time you mentioned what your problem is. A sentence with these words alone and something about line numbers will have made it easier on us.

I am able to see it, no need to change it now. It is just that printing the way you did adds a lot of noise to the image and the tracker. Soon there will be 4K images here to show us how a 32x32 icon looks weird.


See what changing poole.css line 166 to "padding: 0;" does to it. Note that not only these blocks use it, so the change propagates to all code snippets and pre.

You should be able to apply these only to those lines by changing the style settings of the "lineno" class.

Also, I am a bit unsure about changing this theme-wise. I agree that the line numbers column is unnecessarily wide, but going further than that would require some public discussion before any changes.

@quicknir
Copy link
Author

I tried your suggestion, both globally (in poole.css) and just changing lineno. The first is definitely a massive improvement, though there is still huge space left of the line numbers. The second leaves a ton of extra space in front and after. To get the effect of the former without changing everything, I tried to follow something along this website: http://www.minh.io/blog/2015/06/28/jekyll-line-numbers/ and define a lineno-container class, but it didn't seem to work.

I am a total novice at css, all I can speculate is that when line numbers are added, it adds an extra nesting of elements which each get their own padding, which seems to play very poorly with Lanyon's generous spacing settings.

To be clear, my suggestion about merging is not to change anything else. Only to merge changes such that Lanyon plays nicer with line numbers but not to change other behavior (if that is possible). Here are photos corresponding to the two solutions; the one with less padding corresponds to changing poole.css. (I downloaded gimp and cropped the photos this time 👍

screenshot from 2016-02-13 16-48-58
screenshot from 2016-02-13 16-53-56

@bernardosulzbach
Copy link

screenshot from 2016-02-13 20-12-44

Is this what you need?

@quicknir
Copy link
Author

Looks quite ideal, yes. What's your secret?

@bernardosulzbach
Copy link

Well that requires a deposit to be made.

@bernardosulzbach
Copy link

Just kidding, will try to explain it to you here.

@bernardosulzbach
Copy link

My approach was very pragmatic and will not win any code beauty contest, so I wouldn't use it myself (and I don't like the results as much as you do).

I cannot give you the proper changesets and that may require some CSS writing on your own.

Essentially, the left td needs the following

width: 1px;
word-break: keep-all;
padding: .4rem .6rem;

@bernardosulzbach
Copy link

Set its width to too-small-for-business. Do not allow breaking (so line 10 would become two lines, 1 and 0). Padding like that because my eyes approved it.

Done live in Chrome 48.0.2564.97 (64-bit), but not really browser-specific as far as I see it.

screenshot from 2016-02-13 20-21-26

@quicknir
Copy link
Author

Alright, cool, I managed to get some CSS out that at least is allowing me to control these things. If you think you have something good that doesn't affect anything else, it might be good to upstream so that other can benefit. I can try to push something as well but I suspect that my CSS is going to be unacceptable as I have no idea what I'm doing. Thanks a lot for your help with this.

@bernardosulzbach
Copy link

I edited this live on Chrome, I did not touch the CSS files, otherwise I would have opened a PR for you. Organizing it so that these properties only affect these specific code blocks is the complicated part which I will not go through because I have no interest on. I helped you so far by showing what you needed to change, the rest of the way is someone else's job.

@quicknir
Copy link
Author

Alright, once I have this done in a way that works for me I'll do my best to upstream. Thanks again for your help.

@bernardosulzbach
Copy link

If I hadn't to work I or if I would reuse it myself I would do it for you, but I've done my part on this already. Good luck.

geekpradd pushed a commit to geekpradd/geekpradd.github.io that referenced this issue May 15, 2021
add wide screen configuration by adding a `max_width` config in `_config.yml`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
v1.2.0
  
Awaiting triage
Development

No branches or pull requests

2 participants