Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add line number #4

wants to merge 1 commit into


None yet
4 participants

SineSwiper commented Apr 26, 2012

Untested, but the code looks sound.

What's this for?


SineSwiper replied Apr 30, 2012

Notepad++ color syntax fixing. I have no idea how you can even get away with a bareword of '/api'

dgl commented on a4813a0 Apr 30, 2012

I'm not sure this does what you think it does -- the files the matcher are searching on are concatenated chunks of multiple files (called "slabs" in the code) for performance reasons, you'll be counting the lines in these large (10MB) chunks.

There's two ways counting lines could be implemented (one rather hacky):

  • Less hacky: Move the zrevrangebyscore lookup from the client code into the matcher (ideally done in the background so it doesn't slow down the matching).
  • There is special marker used between the files in a slab, it would be possible to search for this and count from that.

As for testing running tools/run-test should give a test copy using the included fakecpan, so it should be easy to test this, let me know if it isn't.


SineSwiper replied Apr 30, 2012

Hmmm, if zset / $file is the slab name, then yes, the bin/cpangrep-matcher code is counting the wrong thing.

Move the zrevrangebyscore lookup: One, does the client code benefit from having line numbers in the result? Also, where is the matcher? Are you talking about lib/WWW/CPANGrep.pm? Would it actually have access to the full text of individual files? (As parsing newlines from fragments doesn't do me any good.)

There is special marker used between the files in a slab: Actually, that doesn't sound all that hacky, especially since you put that marker in place for a reason. It would only change a few more lines of code from within bin/cpangrep-matcher. (Find last marker; if result then $linenum = 0, $fragment = pos to end.) What is the marker?

So, your standard perl Makefile.pl && dmake test would work? Does this work on Win32? (Just so happens to be the OS for my code editing laptop, even though most of it gets loaded on LAMP stuff.)


dgl commented Apr 30, 2012

Yes I agree using the marker is probably easiest, I think actually there are benefits to moving the code using zrevrangebyscore (code I mean to be precise is
https://github.com/dgl/cpangrep/blob/master/lib/WWW/CPANGrep/Search.pm#L163) to the matcher (e.g. you mentioned line: as a filter in your other request).

You'll need a UNIX system (tested on Linux, probably works on OS X), I suspect the use of IO::AIO's mmap support means it won't work on Windows, sorry. It's not a case of make test, but a separate script, see the readme for full details.

(As for working around bugs in your editor's syntax highlighting, no thanks, fix your editor.)


dgl commented Apr 30, 2012

The marker is here ATM: https://github.com/dgl/cpangrep/blob/master/lib/WWW/CPANGrep/Slab/Writer.pm#L8 please extract to a constant somewhere if you do use it (refactoring cpangrep-matcher to be a little more modular might be nice too... ;) )


tsibley commented Apr 18, 2013

I added matching line number calculation to the matcher so I could link excerpts directly to the source and just now found this issue. :) I'll push a pull request later today.

@tsibley tsibley referenced this pull request Apr 19, 2013


Link excerpts to source #28


tsibley commented Apr 28, 2013

This pull request can be closed. Line numbers are added by pull #28 which is merged.

This pull request can be closed.

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