Skip to content

Use pfgrep instead of QShell grep when possible #2429

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,5 @@ Thanks so much to everyone [who has contributed](https://github.com/codefori/vsc
* [@marcin-ogon](https://github.com/marcin-ogon)
* [@Detrytus59](https://github.com/Detrytus59)
* [@janfh](https://github.com/janfh)
* [@MohitKambli](https://github.com/MohitKambli)
* [@MohitKambli](https://github.com/MohitKambli)
* [@NattyNarwhal](https://github.com/NattyNarwhal)
3 changes: 2 additions & 1 deletion src/api/IBMi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const remoteApps = [ // All names MUST also be defined as key in 'remoteFeatures
},
{
path: `/QOpenSys/pkgs/bin/`,
names: [`git`, `grep`, `tn5250`, `md5sum`, `bash`, `chsh`, `stat`, `sort`, `tar`, `ls`, `find`]
names: [`git`, `grep`, `tn5250`, `pfgrep`, `md5sum`, `bash`, `chsh`, `stat`, `sort`, `tar`, `ls`, `find`]
},
{
path: `/QSYS.LIB/`,
Expand Down Expand Up @@ -200,6 +200,7 @@ export default class IBMi {
this.remoteFeatures = {
git: undefined,
grep: undefined,
pfgrep: undefined,
tn5250: undefined,
setccsid: undefined,
md5sum: undefined,
Expand Down
27 changes: 21 additions & 6 deletions src/api/Search.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as path from 'path';
import { GetMemberInfo } from './components/getMemberInfo';
import { IBMiMember, SearchHit, SearchResults } from './types';
import { IBMiMember, SearchHit, SearchResults, CommandResult } from '../typings';
import { Tools } from './Tools';
import IBMi from './IBMi';

Expand All @@ -13,6 +13,8 @@ export namespace Search {
let detailedMembers: IBMiMember[]|undefined;
let memberFilter: string|undefined;

const pfgrep = connection.remoteFeatures.pfgrep;

if (typeof members === `string`) {
memberFilter = connection.sysNameInAmerican(`${members}.MBR`);
} else
Expand All @@ -29,10 +31,23 @@ export namespace Search {
const asp = await connection.lookupLibraryIAsp(library);

// Then search the members
const result = await connection.sendQsh({
command: `/usr/bin/grep -inHR -F "${sanitizeSearchTerm(searchTerm)}" ${memberFilter}`,
directory: connection.sysNameInAmerican(`${asp ? `/${asp}` : ``}/QSYS.LIB/${library}.LIB/${sourceFile}.FILE`)
});
var result: CommandResult | undefined = undefined;
if (pfgrep) {
// pfgrep vs. qshell grep difference: uses -r for recursion instead of -R
// (GNU/BSD grep treat them the same); we don't use recursion yet though...
// older versions before 0.4 need -t to trim whitespace, 0.4 inverts the flag
const command = `${pfgrep} -inHr -F "${sanitizeSearchTerm(searchTerm)}" ${memberFilter}`;
result = await connection.sendCommand({
command: command,
directory: connection.sysNameInAmerican(`${asp ? `/${asp}` : ``}/QSYS.LIB/${library}.LIB/${sourceFile}.FILE`)
});
} else {
const command = `/usr/bin/grep -inHR -F "${sanitizeSearchTerm(searchTerm)}" ${memberFilter}`;
result = await connection.sendQsh({
command: command,
directory: connection.sysNameInAmerican(`${asp ? `/${asp}` : ``}/QSYS.LIB/${library}.LIB/${sourceFile}.FILE`)
});
}

if (!result.stderr) {
let hits = parseGrepOutput(
Expand Down Expand Up @@ -206,4 +221,4 @@ export namespace Search {
}
return index;
}
}
}
15 changes: 15 additions & 0 deletions src/api/tests/suites/search.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,19 @@ describe('Search Tests', {concurrent: true}, () => {
expect(checkNames(result.hits.map(hit => hit.path.split("/").at(-1)!))).toBe(true);
expect(result.hits.every(hit => !hit.path.endsWith(`MBR`))).toBe(true);
});

it('Filtered members list search', async () => {
const pfgrep = connection.remoteFeatures.pfgrep;
// This test only needs to run if pfgrep is installed
if (pfgrep) {
const resultPfgrep = await Search.searchMembers(connection, "QSYSINC", "QRPGLESRC", "IBM", "CMRPG");
connection.remoteFeatures.pfgrep = undefined;
const resultQsh = await Search.searchMembers(connection, "QSYSINC", "QRPGLESRC", "IBM", "CMRPG");
connection.remoteFeatures.pfgrep = pfgrep;
// XXX: Do a deep equals here (without having to reimplement one)
expect(resultPfgrep.hits[0].lines[0] == resultQsh.hits[0].lines[0]);
} else {
expect(true)
}
});
});
4 changes: 3 additions & 1 deletion src/testing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,16 @@ import { StorageSuite } from "./storage";
import { TestSuitesTreeProvider } from "./testCasesTree";
import { ToolsSuite } from "./tools";
import { Server } from "../typings";
import { SearchSuite } from "./search";

const suites: TestSuite[] = [
ActionSuite,
ContentSuite,
DeployToolsSuite,
ToolsSuite,
StorageSuite,
EncodingSuite
EncodingSuite,
SearchSuite
]

export type TestSuite = {
Expand Down
83 changes: 83 additions & 0 deletions src/testing/search.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import assert from "assert";
import { TestSuite } from ".";
import { parseFilter } from "../api/Filter";
import { Search } from "../api/Search";
import { instance } from "../instantiate";

export const SearchSuite: TestSuite = {
name: `Search API tests`,
tests: [
{
name: "Single member search", test: async () => {
const connection = instance.getConnection()!;
const result = await Search.searchMembers(connection, "QSYSINC", "QRPGLESRC", "IBM", "CMRPG");
assert.strictEqual(result.term, "IBM");
assert.strictEqual(result.hits.length, 1);
const [hit] = result.hits;
assert.strictEqual(hit.lines.length, 3);

const checkLine = (index: number, expectedNumber: number) => {
assert.strictEqual(hit.lines[index].number, expectedNumber);
assert.ok(hit.lines[index].content.includes(result.term));
}

checkLine(0, 7);
checkLine(1, 11);
checkLine(2, 13);
}
},
{
name: "Generic name search", test: async () => {
const connection = instance.getConnection()!;
const memberFilter = "E*";
const filter = parseFilter(memberFilter);
const result = await Search.searchMembers(connection, "QSYSINC", "QRPGLESRC", "IBM", memberFilter);
assert.ok(result.hits.every(hit => filter.test(hit.path.split("/").at(-1)!)));
assert.ok(result.hits.every(hit => !hit.path.endsWith(`MBR`)));
}
},
{
name: "Filtered members list search", test: async () => {
const connection = instance.getConnection()!;
const library = "QSYSINC";
const sourceFile = "QRPGLESRC";
// Be stricter in the filter to try to make sure we have six results
const memberFilter = "SQL*,T*";
const filter = parseFilter(memberFilter);
const checkNames = (names: string[]) => names.every(filter.test);

const members = await getConnection().getContent().getMemberList({ library, sourceFile, members: memberFilter });
assert.ok(checkNames(members.map(member => member.name)));

const result = await Search.searchMembers(connection, "QSYSINC", "QRPGLESRC", "SQL", members);
assert.strictEqual(result.hits.length, 6);
assert.ok(checkNames(result.hits.map(hit => hit.path.split("/").at(-1)!)));
assert.ok(result.hits.every(hit => !hit.path.endsWith(`MBR`)));
}
},
{
name: "pfgrep vs. qsh grep equivalency", test: async () => {
const connection = instance.getConnection()!;
const pfgrep = connection.remoteFeatures.pfgrep;
// This test only needs to run if pfgrep is installed
if (pfgrep) {
const resultPfgrep = await Search.searchMembers(connection, "QSYSINC", "QRPGLESRC", "IBM", "CMRPG");
getConnection().remoteFeatures.pfgrep = undefined;
const resultQsh = await Search.searchMembers(connection, "QSYSINC", "QRPGLESRC", "IBM", "CMRPG");
getConnection().remoteFeatures.pfgrep = pfgrep;
assert.deepEqual(resultPfgrep, resultQsh);
} else {
assert.ok(true)
}
}
}
]
}

function getConnection() {
const connection = instance.getConnection();
if (!connection) {
throw Error("Cannot run test: no connection")
}
return connection;
}
Loading