Add support for double-extension action matching - #3315
Conversation
sebjulliand
left a comment
There was a problem hiding this comment.
Thanks for looking into this @yuvalnn !
I left a few suggestions for code clarity.
However, the logic you implemented needs to be changed a bit, regarding the initial need.
With this PR, I'm still able to run an RPGLE action on a .pgm.rpgle file. So, i'll be presented with both "create a module" and "create a program" action on a file that I want to compile only into a program.
The logic should be something like:
-> if the action extension ends with pgm.xxx, then target file path must end with .pgm.xxx to be a valid target.
-> if the action extension is only xxx, then files ending with pgm.xxx must not be valid targets.
-> All of the above is only valid for local files or IFS files. For the other target, strictly checking the extension (or fragment, for objects) is enough
One way to check the multi-part extensions would be to use split(".") on both the action extension and the target file name and use these arrays to check the target validity.
Something along these lines:
if (["file", "streamfile"].includes(target.uri.scheme)) {
//Check all extension parts for workspace files or IFS files
const parts = path.parse(target.uri.path).base.split(".").map(t => t.toUpperCase());
parts.splice(0, 1);
for (const e of extensions.map(ext => ext.split('.'))) {
//every parts must match
if (e.every((val, index) => val === parts[index])) {
return true;
}
}
}
else {
//Check extension only for
return [target.extension.toUpperCase(), target.fragment.toUpperCase()]
.filter(Tools.distinct)
.some(e => extensions.includes(e));
}
return false;Hope this helps 😄
| if (action.type !== scheme) return false; | ||
|
|
||
| // action isn't cleared to run on protected targets and some of them are | ||
| if (!action.runOnProtected && targets.some(t => t.protected)) return false; | ||
|
|
||
| return targets.every((t) => targetMatchesExtensions(t, action.extensions)); |
There was a problem hiding this comment.
You can fuse all the checks in one statement; that will avoid intermediate returns.
| if (action.type !== scheme) return false; | |
| // action isn't cleared to run on protected targets and some of them are | |
| if (!action.runOnProtected && targets.some(t => t.protected)) return false; | |
| return targets.every((t) => targetMatchesExtensions(t, action.extensions)); | |
| return action.type === scheme && | |
| // action isn't cleared to run on protected targets and some of them are | |
| (action.runOnProtected || !targets.some(t => t.protected)) && | |
| targets.every((t) => targetMatchesExtensions(t, action.extensions)); |
|
|
||
| export function targetMatchesExtensions(target: ActionTarget, extensions?: string[]): boolean { | ||
| // action has no extension requirements, or is global | ||
| if (!extensions || extensions.every(e => !e) || extensions.includes("GLOBAL")) return true; |
There was a problem hiding this comment.
Checking if extensions is truthy or empty will be less confusing than doing it with extensions.every(e => !e)
| if (!extensions || extensions.every(e => !e) || extensions.includes("GLOBAL")) return true; | |
| if (!extensions?.length || extensions.includes("GLOBAL")) return true; |
| // action has no extension requirements, or is global | ||
| if (!extensions || extensions.every(e => !e) || extensions.includes("GLOBAL")) return true; | ||
|
|
||
| const targetExtParts = [target.extension.toUpperCase(), target.fragment.toUpperCase()]; |
There was a problem hiding this comment.
Filter on distinct and filter out empty string here to avoid unwanted iterations in the for loop after.
| const targetExtParts = [target.extension.toUpperCase(), target.fragment.toUpperCase()]; | |
| const targetExtParts = [target.extension.toUpperCase(), target.fragment.toUpperCase()] | |
| .filter(Boolean) | |
| .filter(Tools.distinct); |
Fixes #2741
Changes
How to test this PR
Checklist
console.logs I added