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

Add line numbers in sphinx.ext.viewdoc #6319

Merged
merged 15 commits into from
Jul 28, 2023

Conversation

benkrikler
Copy link
Contributor

Subject: Add an option to the viewcode extension to include line-numbering in the output

Feature or Bugfix

  • Feature
  • (and included in here, I believe is potentially a Bugfix; this wasn't the original intention, so I could split it out)

Purpose

I felt that it could help a project I was working on if I the source code as highlighted by the viewcode extension could include line numbers. When I looked into the viewcode source, it seemed relatively straightforward to add this, and so as a way to learn a bit about the internals of sphinx I took a go at it.
At that point, it seemed like it could be something of use / interest and worth contributing back to the main repository. I'm sorry that this hasn't followed what I believe is the normal approach to adding a feature (I read the Contributing guidelines, and whilst it didn't look like a hard rule, I think I should have opened an issue first)!

Detail

Along the way though, I noticed that, at least in my browser (Firefox 66.0.2) the extra div inserted around the [docs] link in the highlighted sourcecode output was adding an extra line intermittently (this was with tag v2.0.1 of Sphinx). The div only seems to have served to add a css class, so I have moved that css class to the <a> element instead and removed the encasing div. I appreciate that might be a bit drastic (for example, styles might be selecting on the div), but it did fix the issue. It seems #4701 and #4709 concerned this problem as well, but as far as I could tell the fix as implemented has only partially worked.

Relates

@codecov
Copy link

codecov bot commented Apr 23, 2019

Codecov Report

Patch coverage: 96.66% and project coverage change: -2.82% ⚠️

Comparison is base (2c0b81d) 86.99% compared to head (7c8576d) 84.18%.

❗ Current head 7c8576d differs from pull request most recent head 49819bb. Consider uploading reports for the commit 49819bb to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6319      +/-   ##
==========================================
- Coverage   86.99%   84.18%   -2.82%     
==========================================
  Files         276      274       -2     
  Lines       52443    39314   -13129     
  Branches     9208     5835    -3373     
==========================================
- Hits        45625    33098   -12527     
+ Misses       5163     4900     -263     
+ Partials     1655     1316     -339     
Files Changed Coverage Δ
sphinx/ext/viewcode.py 82.73% <95.23%> (+1.57%) ⬆️
tests/test_ext_viewcode.py 100.00% <100.00%> (+3.07%) ⬆️

... and 315 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

for name, docname in used.items():
type, start, end = tags[name]
backlink = urito(pagename, docname) + '#' + refname + '.' + name
lines[start] = (
'<div class="viewcode-block" id="%s"><a class="viewcode-back" '
Copy link
Member

Choose a reason for hiding this comment

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

This <div> tag is used for highlighting the source code when user came from description:
スクリーンショット 2019-04-26 1 15 02

I think this removal disables the highlights. This is not a bug fix.

@@ -141,6 +141,23 @@ def missing_reference(app, env, node, contnode):
return None


def split_code_lines(lines):
Copy link
Member

Choose a reason for hiding this comment

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

I feel this code is a bit tricky. To be our code robust, we need to make a custom Formatter for pygments. I'll take a look in this weekend.

@tk0miya tk0miya added extensions type:proposal a feature suggestion labels Apr 25, 2019
@tk0miya tk0miya added this to the 2.1.0 milestone Apr 25, 2019
@tk0miya tk0miya modified the milestones: 2.1.0, 2.2.0 May 31, 2019
@tk0miya tk0miya modified the milestones: 2.2.0, 2.3.0 Aug 18, 2019
@tk0miya tk0miya modified the milestones: 2.3.0, 2.4.0 Dec 8, 2019
@tk0miya tk0miya modified the milestones: 2.4.0, 3.0.0 Feb 7, 2020
@tk0miya tk0miya modified the milestones: 3.0.0, 3.1.0 Mar 14, 2020
@tk0miya tk0miya modified the milestones: 3.1.0, 4.0.0 May 30, 2020
@tk0miya tk0miya modified the milestones: 4.0.0, 4.1.0 Apr 4, 2021
@tk0miya tk0miya modified the milestones: 4.1.0, 4.2.0 Jul 10, 2021
@tk0miya tk0miya modified the milestones: 4.2.0, 4.3.0 Sep 12, 2021
@tk0miya tk0miya modified the milestones: 4.3.0, 4.4.0 Nov 8, 2021
@tk0miya tk0miya modified the milestones: 4.4.0, 4.5.0 Jan 15, 2022
@tk0miya tk0miya modified the milestones: 4.5.0, 5.0.0 Mar 27, 2022
@tk0miya tk0miya removed this from the 5.0.0 milestone May 2, 2022
@tk0miya tk0miya added this to the 5.x milestone May 2, 2022
@AA-Turner AA-Turner modified the milestones: 5.x, some future version May 23, 2022
@AA-Turner AA-Turner changed the base branch from master to 5.x October 5, 2022 13:31
@AA-Turner AA-Turner changed the base branch from 5.x to master October 16, 2022 15:22
@AA-Turner AA-Turner changed the title viewdoc linenos Add line numbers in sphinx.ext.viewdoc Jul 28, 2023
@AA-Turner AA-Turner merged commit 762ed85 into sphinx-doc:master Jul 28, 2023
26 checks passed
@AA-Turner
Copy link
Member

Thanks @benkrikler!

A

@AA-Turner AA-Turner modified the milestones: some future version, 7.2.0 Aug 11, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
extensions type:proposal a feature suggestion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants