feat(stdlib): extract function #587
Conversation
|
Implemented new functions for |
| @@ -21,7 +21,7 @@ pub fun replace_regex(source: Text, search: Text, replace: Text, extended: Bool | |||
| // contains "GNU sed". | |||
| $ re='\bCopyright\b.+\bFree Software Foundation\b'; [[ \$(sed --version 2>/dev/null) =~ \$re ]] $ | |||
| let flag = status == 0 then "-r" else "-E" | |||
| return $ echo "{source}" | sed {flag} -e "s/{search}/{replace}/g" $ | |||
| return $ echo "{source}" | sed "{flag}" -e "s/{search}/{replace}/g" $ | |||
There was a problem hiding this comment.
I just noticed the issue with shellcheck, I don't think that we need a PR just for this
src/tests/stdlib/extract.ab
Outdated
| let package = tmpdir + "/" + "filename.tar.gz" | ||
|
|
||
| if (extract(package, tmpdir)) { | ||
| if dir_exist(tmpdir + "/" + tmpdir) { |
There was a problem hiding this comment.
the package include the full path for this reason the check is like that
Co-authored-by: Phoenix Himself <[email protected]>
Co-authored-by: Phoenix Himself <[email protected]>
| let flag = status == 0 then "-r" else "-E" | ||
| output = $ echo "{source}" | sed "{flag}" -e "{search}" $ | ||
| } else { | ||
| output = $ echo "{source}" | sed -e "{search}" $ |
There was a problem hiding this comment.
Need to use sed -n (suppress automatic printing), and /.../p to print matching lines:
...
output = $ echo "{source}" | sed "{flag}" -ne "/{search}/p" $
} else {
output = $ echo "{source}" | sed -ne "/{search}/p" $
...
| } else { | ||
| output = $ echo "{source}" | sed -e "{search}" $ | ||
| } | ||
| if output == "" { |
There was a problem hiding this comment.
Need to return true if sed output is not empty:
if output != "" {
return true
}
| contains_multiple(path, [".xz"]): trust $xz --decompress "{path}"$ | ||
| contains_multiple(path, [".7z"]): trust $7z -y "{path}" -o "{target}"$ | ||
| contains_multiple(path, [".zip", ".war", ".jar"]): trust $unzip "{path}" -d "{target}"$ | ||
| contains_any_regex(path, ["/.tar.bz2$/g", "/.tbz$/g", "/.tbz2$/g"]): $ tar xvjf "{path}" -C "{target}" $? |
There was a problem hiding this comment.
Several suggestions: (i) forward slashes should be moved into the standard library function, (ii) you don't need the "global" specifier /.../g at all, (iii) \. should be escaped, and (iv) you can merge the multiple extensions into a single expression, with escaped grouping operators \(, \| and \) (as we're using basic not extended sed regular expressions for maximum compatibility):
contains_regex(path, "\(\.tar\.bz2\|\.tbz\|\.tbz2\)$"): $ tar xvjf "{path}" -C "{target}" $?
There was a problem hiding this comment.
In which case, we don't really need a contains_any_regex function after all.
There was a problem hiding this comment.
I was thinking that is better to leave to the user how to build the regular expression. In sed you can use also # or other symbols as separatore instead of /, also you can use different endings instead of /g so as it is a regex solution I was thinking that is better to leave to the user how to do what they prefer.
There was a problem hiding this comment.
Respectfully, I disagree. But I'll have to spend some time composing my reasoning. Will get back to you this evening.
| echo "{label}: {result}" | ||
| } | ||
|
|
||
| main { |
There was a problem hiding this comment.
Please remove / and /g:
main {
test_multiple("Empty", "Hello World", [])
test_multiple("None", "Hello World", ["Other", "Other$"])
test_multiple("Right", "Hello World", ["Other", "World$"])
test_multiple("Left", "Hello World", ["^Hello", "Other"])
test_multiple("Both", "Hello World", ["^Hello", "$World"])
}
| @@ -0,0 +1,7 @@ | |||
| import { contains_regex } from "std/text" | |||
|
|
|||
| main { | |||
|
I have some doubts about removing |
Ref: #449
So I implemented the extract function in amber, required a new function
contains_multiple