Project:API
Version:6.x-1.x-dev
Component:Code
Category:feature request
Priority:major
Assigned:jhodgdon
Status:closed (fixed)

Issue Summary

When viewing files on api.drupal.org, it would sometimes be beneficial to have the line numbers by the left margin, for comparing with diff's, error messages, or general code orientation. (If the source code of functions etc. could have line number from their parent file as well, that would be awesome, but would probably also require some more coding.)

Comments

#1

I agree. I will give a shot at implementing that if I have some free time this week.

#2

Version:5.x-1.0» 6.x-1.x-dev

Bump...
I've strongly agree with this request. It'd be very useful (to me and presumably others) in finding the correct lines.

#3

Status:active» needs review

Here's a patch.

It modifies the parser that builds the code index by sticking in a (line) number after each line break (PHP_EOL). I've wrapped the method in a function so it's less intrusion and wrapped the line number format in a function to more easily modify how we'd like to format the line number. I'm not sure if we have to go as far as registering the line number format function in the theme registry though.. =P

The drawback is that the line numbers are embedded into the code. Is that tolerable?

Most of the other online sites that post code snippets use ordered lists to put in line numbers. Their drawback is that the code is actually broken up into separate lines. Styles that span across multiple lines are closed at the end of each line and reopened at the start of the next line.

I can look at how to put a list of line numbers to the left of the code. Maybe float them in an HTML element to the left of the code element. Maybe something else. Suggestions?

AttachmentSize
227449-3.patch 2.25 KB

#4

Status:needs review» needs work

Copy/paste definitely needs to work. An ordered list won't work since we will rarely start at line 1. And this should not be mixed into the code in the database, it should be rendered on output.

#5

Priority:normal» major

Bump. When we tell where functions are, we tell what the line number is in the file. Then if you click on the file link, you can't see the line number, so the only way to find the function would be to control-F search for the function name I guess? We definitely need line numbers for files.

I'm less certain that we need them for the code shown on function pages.

#6

And regarding the copy/paste, I wonder if it would be possible to have a JavaScript link and/or a URL query string that would turn their display on/off (maybe have the default be off) (we would need the query string so that it would work for people without JavaScript)? Maybe too complicated...

#7

I did some testing today.

It appears that in Internet Explorer 7, as well as Firefox/Chrome on Ubuntu, if you copy/paste from an OL (numbered list), the numbers are not included in the paste (I tested by pasting into Notepad on Windows, and Emacs on Ubuntu). So an ordered list should work for line numbers.

There is a deprecated "start" attribute that can be added to OL lists to indicate the starting number, and a deprecated "value" attribute that can be added to LI lists to indicate the starting number (on the first LI). Although both are deprecated, w3schools.com indicates they are supported in all major browsers. They are both in theory supposed to be taken over by CSS attributes, but there doesn't appear to be a CSS attribute that actually does this, so these deprecated attributes are the only option (at least according to w3schools.com, which I find to be generally trustworthy in these types of things).

Regarding #4, the above would take care of the line number starting issue.

Regarding the other part of #4, I'm not sure about what drumm said regarding rendering this at output time vs. saving the OL in the database.... What is being saved in the database now is the output of api_format_php(), not the pure code anyway. I don't see a philosophical problem in formatting it as a UL list in addition to all of the other formatting that is being done?

Also, api_format_php() is currently returning a single PRE element. That won't really work if we are converting it to an OL. We would instead need to split on newlines, turn each line into a LI, and wrap each LI in a PRE I think? So it seems like api_format_php() is the right place to do this? I think it would need an additional argument for the starting line, and one to turn line numbers on/off (defaulting to off for backwards compatibility).

Thoughts?

#8

#5: Could clicking the file link jump directly to #something. That would remove the need to hunt for lines or find identifiers. But that's another issue.

#6: The line numbers should just always be on, and not be intrusive enough for people to want to turn them off.

#7: HTML5 actually brings start back, http://www.whatwg.org/specs/web-apps/current-work/multipage/grouping-con....

It might be worth searching around other projects that have line numbering for code and seeing what pros/cons they found. I see GitHub does a big two-column table. Also might be worth looking for Drupal modules to help. No sense in solving the same problem if someone has done a decent job of it.

This would be a good time for unit tests, to make sure the formatted code, with HTML stripped, is the same as the original code. (api_format_php() does use token_get_all(), which could move to using Grammar Parser objects. There was some code for that in the original move, but I wanted to do one thing at a time.)

I do think we should do as much as possible at parse time and store it in the database, so viewing pages is as fast as possible. The exception is linking, the destination of the link might not be parsed yet.

#9

RE #8 - here's a quick survey of what some other sites with line numbers are doing:

- pastebin.com ==> uses OL [each line of code is an LI, copy/paste works]
- gist.github.com (another pastebin type site) ==> uses a table [all line numbers in one TD, then all lines of code in another TD]. Copy/paste works.
- github.com (code listings) ==> uses a table like gist.github.com
- sourceforge.net ==> could not find a repository viewer?
- drupalcode.org ==> BAD - puts the line numbers straight in with the code, so you cannot copy/paste.
- trac (such as core.trac.wordpress.org) ==> BAD - line number is one TD, line is next TD, next line is a new TR. This method means the numbers copy/paste with the text, which is bad.
- lrx.linux.no ==> see drupalcode.org (bad)
- opensolaris.org (uses opengrok) ==> see drupalcode.org (bad)
- Google code ==> Uses two divs -- one containing all the line numbers, one with all the code (copy/paste works)

So... It looks like everyone just did whatever they thought of, and there aren't any standards.

The other ideas in #8 all sound good. I think the #5 idea of having the file link jump to the right line in the file would be easiest to accomplish if we used an OL and put an ID on each LI, so I'm inclined to go with that.

#10

Regarding using another Drupal module... I did a search for modules with "code formatter" or "code viewer" keywords on d.o. I found:
http://drupal.org/project/codefilter -- this is the PHP highlighting filter used for CODE tags on d.o. It just calls the PHP highlight_string() function, and doesn't do line numbers.
http://drupal.org/project/widecode - this is just a JavaScript thing to widen preformatted blocks, and it doesn't do any other formatting.
http://drupal.org/project/prettify - this uses the Google prettifier, which uses JavaScript to colorize code in PRE blocks -- see http://google-code-prettify.googlecode.com/svn/trunk/README.html

So I don't think there is anything particularly useful to us. We can't really use someone else's prettifier anyway, probably, because of the links?

#11

Assigned to:Anonymous» jhodgdon

I think this is ready to take on, and I have some time today.

#12

In looking at this, I realized that making the links from a function to the file go straight to the code line are not going to work well. The reason is that they go to the file page, which has the source code in a hidden block, so a #ID fragment will not actually take you to that line of code. So that is not going to work.

#13

Here's one patch using OL for the line numbers. I don't think I really like the result -- the line numbers are the same color as the code, and I think it makes the code kind of hard to read. I don't really think there's a way to make OL numbering a different color from the base text in CSS, so I don't think can be changed. See screen shot and patch.

I'll try again with a DIV.

AttachmentSize
227449-use-ol.patch 3.07 KB
ol-line-numbers.png 22.1 KB

#14

Status:needs work» needs review

Here's a better patch. It uses a table. I put in some colors for the default CSS for the line numbers; easy for a site to override if they wanted to.

Any thoughts?

AttachmentSize
227449-use-table.patch 2.72 KB
table-line-numbers.png 35.28 KB

#15

In IRC drumm suggested a different CSS scheme with white background, 666666 foreground for the line numbers (which is the same as comments). Here's a screen shot.

AttachmentSize
line-numbers-666666.png 19.83 KB

#16

Status:needs review» fixed

Committed with that CSS from #15.
http://drupalcode.org/project/api.git/commit/624ab54

#17

Hurray! Fixed before its 3 year anniversary! :D Now the code just needs to go live... :p

#18

Status:fixed» closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

#19

Note: This fix did not work so well on the api.drupal.org site. See #1433512: Line numbers not lining up right (follow-up issue)

nobody click here