Uh oh!
There was an error while loading. Please reload this page.
Added 'rglob' function to stdlib. - #242
Conversation
codecov-io
commented
Sep 28, 2017
Codecov Report
@@ Coverage Diff @@## master #242 +/- ##
==========================================
- Coverage 56.96% 56.94% -0.02%
==========================================
Files 26 25 -1 Lines 4403 4401 -2 ==========================================
- Hits 2508 2506 -2
Misses 1653 1653 Partials 242 242
Continue to review full report at Codecov.
|
i4ki
left a comment
There was a problem hiding this comment.
overall code is nice. If this test approach standardize, we can make a nice lib to easy other tests.
Maybe we could add this function to a proper 'module' directory, maybe called 'filepath' (like Go) or 'path' ?
Then, using it will be something like:
import filepath/rglob
results <= rglob("*")
We need to always make sure the script do not abort in stdlib functions.
| @@ -0,0 +1,29 @@ | |||
| fn dir_glob(dir, pattern, results) { | |||
| files <= find $dir -maxdepth 1 -mindepth 1 | |||
There was a problem hiding this comment.
stdlib functions should never abort the script. We must treat (or ignore in some cases) all errors.
| @@ -0,0 +1,29 @@ | |||
| fn dir_glob(dir, pattern, results) { | |||
| files <= find $dir -maxdepth 1 -mindepth 1 | |||
There was a problem hiding this comment.
you can optimize this a lot passing -type d to get only the directories.
| files <= split($files, "\n") | ||
| for f in $files { | ||
| _, status <= test -d $f |
There was a problem hiding this comment.
if asking only directories to find then this check could be skipped.
What do you think?
| } | ||
| fn rglob(pattern) { | ||
| working_dir <= pwd |
There was a problem hiding this comment.
I dont think this is the right way to handle the rglob. It will only work in the current directory? I'm not saying we must add support to the '/path/**' syntax, but maybe the signature could be rglob(dir, pattern).
What do you guys think?
| _, _ <= mkdir $hello_dir | ||
| _, _ <= mkdir $world_dir | ||
| honda_txt = $hello_dir+"/honda.txt" |
| mkdir $hello_dir | ||
| mkdir $world_dir | ||
| honda_txt = $hello_dir+"/honda.txt" |
i4ki
commented
Oct 14, 2017
@vitorarins Version 1 of unix had a program called |
@tiago4orion the idea is to remove the builtin glob function then ? |
Awesome! @katcipis @tiago4orion What do you think about creating a new glob that is capable of accepting the two asterisks ( |
i4ki
left a comment
There was a problem hiding this comment.
added something I found when running the rglob tests
| import rglob | ||
| fn setup() { | ||
| temp_dir <= mktemp -d |
| touch $civic_txt | ||
| touch $civic_sh | ||
| return $temp_dir |
There was a problem hiding this comment.
better to return the cleaning function also, something like:
fn clean() {
rm -rf $temp_dir
}
return $temp_dir, $clean
i4ki
commented
Oct 17, 2017
@katcipis I don't think so. I prefer a builtin function that handles ** also, or a function in stdlib.. I sent the link to the unix glob because maybe we can create a helper binary to help the rglob nash function. But maybe this doesn't make sense. |
katcipis
commented
Oct 17, 2017
@tiago4orion If the builtin function will continue to exist I see very little incentive to have a helper binary...not sure in what will exactly it help if we give support on the glob function. |
No description provided.