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

[PATCH] Add line numbers to source code #22

Closed
GoogleCodeExporter opened this issue Apr 19, 2016 · 27 comments
Closed

[PATCH] Add line numbers to source code #22

GoogleCodeExporter opened this issue Apr 19, 2016 · 27 comments

Comments

@GoogleCodeExporter
Copy link

This is an enhancement to have prettify display line numbers adjacent to the 
formatted code.

The basic idea is to wrap the pretty-printed <pre> in a <div> and having a new 
<pre> that 
contains line numbers float left of it. This is somewhat of a bigger 
intervention with the DOM 
of the page, but it looks nice. Additionally, users can easily copy/paste the 
code without having 
line numbers all over it, which I think is quite important.

The code below adds a configuration variable, PR_CREATE_LINE_BOX, to 
enable/disable the line 
box, and a variable to configure the line stepping, defaulting to 5. It also 
adds default styles to 
the included stylesheet.

I have also attached the patch in case the formatting goes wrong.

diff -u prettify_31_Aug_2007/prettify.css 
prettify_31_Aug_2007-changed/prettify.css
--- prettify_31_Aug_2007/prettify.css   2007-05-22 14:20:44.000000000 +0200
+++ prettify_31_Aug_2007-changed/prettify.css   2007-11-09 12:16:15.000000000 
+0100
@@ -12,6 +12,8 @@
 .atv { color: #080; }
 .dec { color: #606; }
 pre.prettyprint { padding: 2px; border: 1px solid #888; }
+div.prettycontainer {}
+pre.prettylines { float: left; padding: 3px; margin: 0px; text-align: right; 
margin-right: 10px; }

 @media print {
   .str { color: #060; }
diff -u prettify_31_Aug_2007/prettify.js 
prettify_31_Aug_2007-changed/prettify.js
--- prettify_31_Aug_2007/prettify.js    2007-08-31 04:21:54.000000000 +0200
+++ prettify_31_Aug_2007-changed/prettify.js    2007-11-09 12:29:13.000000000 +0100
@@ -110,6 +110,11 @@
 /** the number of characters between tab columns */
 var PR_TAB_WIDTH = 8;

+/** whether we should create a separate box with line numbers */
+var PR_CREATE_LINE_BOX = true;
+/** the stepping for line numbers */
+var PR_LINE_STEPPING = 5;
+
 // some string utilities
 function PR_isWordChar(ch) {
   return (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z');
@@ -875,6 +880,29 @@
   return html.join('');
 }

+/** wrap the given pre in a div and prepend a pre containing line numbers
+  * 
+  * @param {Element} pre the pre element to wrap
+  */
+function PR_create_line_box(pre, numLines) {
+  // count line numbers within pre
+  var containerdiv = document.createElement('DIV');
+  containerdiv.setAttribute('class', 'prettycontainer')
+  var linespre = document.createElement('PRE');
+  linespre.setAttribute('class', 'prettylines')
+  var text = '';
+  for (var i=1; i < (numLines + 2); i++) {
+    if (i % PR_LINE_STEPPING == 0) {
+      text += i;
+    }
+    text += "\n";
+  }
+  linespre.innerHTML = text;
+  pre.parentNode.replaceChild(containerdiv, pre);
+  containerdiv.appendChild(linespre);
+  containerdiv.appendChild(pre);
+}
+
 /** pretty print a chunk of code.
   *
   * @param {String} sourceCodeHtml code as html
@@ -967,11 +995,13 @@
           // fetch the content as a snippet of properly escaped HTML.
           // Firefox adds newlines at the end.
           var content = PR_getInnerHtml(cs);
+          // number of lines (for line box)
+          var numLines = content.replace(/[^\n]/g, '').length;
           content = content.replace(/(?:\r\n?|\n)$/, '');

           // do the pretty printing
           var newContent = prettyPrintOne(content);
-
+                   
           // push the prettified html back into the tag.
           if (!PR_isRawContent(cs)) {
             // just replace the old html with the new
@@ -990,6 +1020,11 @@
             pre.innerHTML = newContent;
             // remove the old
             cs.parentNode.replaceChild(pre, cs);
+            // update cs to work with PR_create_line_box below
+            cs = pre;
+          }
+          if (PR_CREATE_LINE_BOX && cs.localName.toLowerCase() == 'pre') {
+            PR_create_line_box(cs, numLines);
           }
         }
       }

Original issue reported on code.google.com by mrtnpr...@googlemail.com on 9 Nov 2007 at 11:34

Attachments:

@GoogleCodeExporter
Copy link
Author

Maybe it's a good idea to create something like this using a list. This would 
allow
us to still copy paste the code easily (without selecting line numbers) and 
would
give the possibility to add row colors and/or row highlighting.

Original comment by Po0ky.yk...@gmail.com on 25 Apr 2008 at 11:11

@GoogleCodeExporter
Copy link
Author

For anyone who's interested, here's a new patch based on Martin Probst's work on
displaying line numbers, with a few improvements:

- improve cross-browser support.
  (tested on IE6, IE7, Opera9.51, Konqueror3.5.9, Firefox3)
- use <table> to arrange linebox and prettified box, so that the layout 
  will still work for browsers with CSS turned off but Javascript turned on.
- support table layout <pre><table>..</table</pre> used in Google Code Hosting.

I also attach a test file containing test cases for line numbers rendering.

Original comment by teohuiming on 9 Jul 2008 at 4:34

Attachments:

@GoogleCodeExporter
Copy link
Author

Rather than try and implement all kinds of decorations, I thought I'd allow 
people to
embed their own.

I implemented a scheme to allow this.  See "nocode" spans as described at
http://google-code-prettify.googlecode.com/svn/trunk/README.html

I'm going to mark this fixed, but please reopen if this doesn't meet your need.

Original comment by mikesamuel@gmail.com on 14 Jul 2008 at 11:00

  • Changed state: Fixed

@GoogleCodeExporter
Copy link
Author

Sorry that I didn't respond earlier, but apparently I was not notified of 
changes in this bug. I also cannot see 
how to re-open it (?!). However I'd vote to re-open this, as the proposed 
solution doesn't really work for 
me.

I think the idea of this library is to add decorations to code on the fly using 
JavaScript. Now I consider line 
numbers decorations that don't really belong with the code, so I'd like my 
JavaScript to add them 
automatically, without me having to manually enter them in the code (and in 
quite a verbose syntax using 
<span/>).

Also, the original patch and the enhancement by teohuiming allow users to 
copy/paste the code from the 
snippet without the line numbers (as it is within a different box element), 
which is IMHO an important 
feature not supported by the <span/> method.

Original comment by mrtnpr...@googlemail.com on 14 Sep 2008 at 8:57

@GoogleCodeExporter
Copy link
Author

For reference, you can reopen a bug by changing the "Status" from "Fixed" to 
"New".

Original comment by mikesamuel@gmail.com on 22 Sep 2008 at 9:42

  • Changed state: New

@GoogleCodeExporter
Copy link
Author

i agree with Martin - the original proposed implementation, or the changes by 
Teo,
would be very interesting features.

Original comment by sgbeal@googlemail.com on 21 Feb 2009 at 11:44

@GoogleCodeExporter
Copy link
Author

Try using the <code> tag instead of the <div> tag to make it much more semantic

Original comment by yun4.lit...@gmail.com on 10 Jul 2009 at 3:17

@GoogleCodeExporter
Copy link
Author

An HTML ordered list sounds semantically sound (as the meta line numbers are 
better
coupled closer to the content) and far less complicated:

<code>
    <ol>
        <li>...</li>
        ...
    </ol>
</code>

In a copy/paste operation, browsers often don't copy the item/line numbers
automatically generated. Workarounds for browsers that do may be required.

Original comment by paul.ar...@gmail.com on 10 Mar 2010 at 10:35

@GoogleCodeExporter
Copy link
Author

So there should be a way to get the rewriter to put <li></li> around each line?
If those are in the code already, the system should preserve them.

Original comment by msam...@google.com on 11 Mar 2010 at 4:50

@GoogleCodeExporter
Copy link
Author

The drawback with <ol><li>...</li></ol> is that you cannot easily create a 
stepping (only display a number 
every n lines), can you? The advantage is of course that it is much easier to 
do the counting and aligning the 
lines correctly.

Original comment by mrtnpr...@googlemail.com on 11 Mar 2010 at 8:20

@GoogleCodeExporter
Copy link
Author

Can you explain steppings further? I'm not sure what they are.

I think I straight correlation between the line numbers a compiler/lint/etc 
sees and
what we see is paramount in easing the administrative burden of finding/fixing 
bugs.

Original comment by paul.ar...@gmail.com on 11 Mar 2010 at 2:27

@GoogleCodeExporter
Copy link
Author

With stepping I mean the common practice that you only want every Nth line 
number to be displayed.

E.g.
1. for (x in xs) {
      print('a')
      print('a')
      print('a')
5.   print('a')
      print('a')
      print('a')
      print('a')
10. print('a')
    }

Original comment by mrtnpr...@googlemail.com on 11 Mar 2010 at 6:21

@GoogleCodeExporter
Copy link
Author

Can you do stepping in CSS?
  <ol>
    <li class="step0">
    <li class="step1">
    <li class="step2">
    <li class="step3">
    <li class="step4">
    <li class="step5">
    ...
  </ol>
  <style>
    li.step1, ... li.step4 { list-style-type: none }
  </style>

Original comment by msam...@google.com on 11 Mar 2010 at 6:39

@GoogleCodeExporter
Copy link
Author

Yes

li { list-style-type: none; }

li:first-child, /* 1 */
li:nth-child(5n) { list-style-type: decimal; } /* 5, 10, 15... */

Original comment by paul.ar...@gmail.com on 11 Mar 2010 at 9:22

@GoogleCodeExporter
Copy link
Author

That's much nicer CSS, though it might stomp on list-styles outside 
.prettyprint blocks.

Maybe prefix those selectors with
   .prettyprint > 

I'm unfamiliar with the nth-child pseudo-selector.  Does it work on most 
browsers?

Original comment by msam...@google.com on 11 Mar 2010 at 10:00

@GoogleCodeExporter
Copy link
Author

It was intended as informative not normative.
:)

Looking at the source (prettify.css), pre.prettyprint appears to be the relevant
wrapper class.

nth-child falls under CSS3 selectors (currently at PR Proposed Recommendation
status). Support is better than you'd think: most current browsers. The 
elephant in
the room can be Dean-handled: http://ie7-js.googlecode.com/svn/test/index.html

Original comment by paul.ar...@gmail.com on 11 Mar 2010 at 10:39

@GoogleCodeExporter
Copy link
Author

So I guess the open questions are:

Does prettyprint need to insert tags to separate or delimit line boundaries?
Or just recommend that users delimit their line boundaries with <li> or some 
other
block level tags?

Does prettyprint need to descend into <ol class=prettyprint>?
Or just recommend <li> tags inside <code class=prettyprint>?

Should prettyprint.js include any CSS related to line numbering?

What documentation/FAQs need to be updated?

Original comment by msam...@google.com on 12 Mar 2010 at 1:08

@GoogleCodeExporter
Copy link
Author

I would suggest that prettyprint adds the <ol> and <li> automatically, the 
whole promise of the library is that 
you throw a bunch of plain text within a <pre> tag at it and it will do it's 
thing. Requiring people to add <li>s 
would work against that.

Thus, there should also be CSS included.

Original comment by mrtnpr...@googlemail.com on 12 Mar 2010 at 8:37

@GoogleCodeExporter
Copy link
Author

You really need to take seriously what the user above said:
        "the whole promise of the library is that 
you throw a bunch of plain text within a <pre> tag at it and it will do it's 
thing."

Original comment by flavius...@gmail.com on 22 Mar 2010 at 2:19

@GoogleCodeExporter
Copy link
Author

Agreed in principle, but I need to check what browsers do with <ol> and <li> 
within
<pre> tags.

Original comment by msam...@google.com on 23 Mar 2010 at 6:30

@GoogleCodeExporter
Copy link
Author

IMO an easier solution would be to use two TD columns, with the numbers on the 
left 
and code on the right. This also simplifies copy/paste operations, since you 
can 
select a single TD's contents without also selecting the numbers. Word-wrapping 
might 
be problematic with this approach, however, unless each line is a TR (in which 
case 
the copy/paste benefit goes away again, i think).

Original comment by sgbeal@googlemail.com on 23 Mar 2010 at 6:45

@GoogleCodeExporter
Copy link
Author

sgbeal,

Thanks for the suggestion.  How does copy work around tables when the selection
starts before the table.

Original comment by msam...@google.com on 23 Mar 2010 at 7:02

@GoogleCodeExporter
Copy link
Author

I updated the patch by teohuiming
(http://code.google.com/p/google-code-prettify/issues/detail?id=22#c2) to be 
based on
prettify-3-Dec-2009.zip, fixed some issues I had with it and added a way to 
specify
the starting line number.

Patch set attached. (only for prettify.js; I did some CSS changes too but I 
don't
think any were necessary to make it work. Do use the css patch by teohuiming!)

Original comment by tobivoll...@gmail.com on 4 May 2010 at 8:20

Attachments:

@GoogleCodeExporter
Copy link
Author

I would love to see line numbers included.

tobivollebregt would you be able to repost attaching your patched prettify.js 
and the prettify.css patched by 
teohuiming?... 

bonus points for attaching minified versions also ;-)

Original comment by i.chris....@gmail.com on 25 May 2010 at 6:57

@GoogleCodeExporter
Copy link
Author

Sorry I've been so quiet lately.
I'm going to try to get some free time soonish to work through a backlog of 
bugs.
If this bug is a major issue for you, please respond on 
http://groups.google.com/group/js-
code-prettifier where I'm going to ask people what their priorities are as far 
as bug fixes.

Original comment by mikesamuel@gmail.com on 27 May 2010 at 2:40

@GoogleCodeExporter
Copy link
Author

Candidate implementation up and discussion at 
http://groups.google.com/group/js-code-prettifier/browse_thread/thread/fea5f0528
345f3d1

Original comment by mikesamuel@gmail.com on 19 Jul 2010 at 6:16

  • Changed state: Started

@GoogleCodeExporter
Copy link
Author

The candidate implementation is now stable.

Original comment by mikesamuel@gmail.com on 5 Apr 2011 at 2:54

  • Changed state: Fixed

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

No branches or pull requests

1 participant