Use pfgrep instead of QShell grep when possible#2429
Use pfgrep instead of QShell grep when possible#2429worksofliam merged 9 commits intocodefori:masterfrom
Conversation
This is the main question. I suspect the best we can do is offer a page in our docs (under Tips) specifically for getting |
|
Might want to wait for next version, I'm working on |
|
@NattyNarwhal I don't know if you're interested, but we have a major codebase change in the last week, so it might be worth fixing up this PR. |
|
I'll push out an update to pfgrep this week and rebase. |
|
@NattyNarwhal I think your test cases need to be moved over to the newer https://github.com/codefori/vscode-ibmi/blob/master/src/api/tests/suites/search.test.ts |
|
I did move the search tests over to the API tests suite; should I revert the ones in the non-API tests suite side? |
[pfgrep][pfgrep] is a fast alternative to QShell grep. It aims to be much faster and support the full spectrum of PCRE regular expression functionality. In my testing, searching for the string `Qp0l` in `QSYSINC/H` shows: - qshell grep: ~11 seconds - pfgrep v0.2: ~8 seconds - pfgrep v0.3: ~2 seconds Quite a big improvement! Questions include: - Should VS Code offer to point you to where to download pfgrep? It's not yet in the IBM repository, but we do have our own repo, RPMs available, and it can be built from source. - With faster search, doing things like regular expressions or searching across multiple files/libraries goes from tedious to quite possible. That's out of scope for this PR, but would be quite nice to add. Same deal for regular expressions instead of fixed strings. - If you do need any features in pfgrep, let me know. I can prioritize things that VS Code needs. [pfgrep]: https://github.com/SeidenGroup/pfgrep
By default pfgrep includes the full record, including whitespace. Maybe this should be changed, but this is what it does now; use -t to match semantics of qsh grep. Note that this will need pfgrep v0.3.2; there was a bug in character trimming in previous versions.
We don't have skips I think, so this may be weird.
|
I've rebased this for the ASP handling changes. |
|
@NattyNarwhal Good news - it's time to actually get this merged. I need the PR updated with the latest from |
| directory: connection.sysNameInAmerican(`${asp ? `/${asp}` : ``}/QSYS.LIB/${library}.LIB/${sourceFile}.FILE`) | ||
| }); | ||
| } else { | ||
| const command = `/usr/bin/grep -inHR -F "${sanitizeSearchTerm(searchTerm)}" ${memberFilter}`; |
There was a problem hiding this comment.
Something we need to do in the future is not use the hardcoded grep here, but instead use grep from remoteFeatures.
There was a problem hiding this comment.
Though looking at it, we might have a problem using remote features with two different versions of grep (since they share a name and key). Ignore this for now.
worksofliam
left a comment
There was a problem hiding this comment.
Tests are passing. Thank you and sorry for the wait.
pfgrep is a fast alternative to QShell grep. It aims to be much faster and support the full spectrum of PCRE regular expression functionality.
In my testing, searching for the string
Qp0linQSYSINC/Hshows:Quite a big improvement!
Questions include:
Changes
Description of change here.
How to test this PR
Examples:
Checklist
console.logs I added