Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

Allow directory scanning without recursion flag. #28

Closed
wants to merge 1 commit into from
Closed

Allow directory scanning without recursion flag. #28

wants to merge 1 commit into from

Conversation

markkimsal
Copy link

The replace package will not scan files in any directory unless the
recursion option is set. This has unintended consequences if only
files in one directory should be affected. This patch allows
scanning the initial path parameter if it is a directory and keeps
track of the recurse depth with an extra parameter.

The replace package will not scan files in any directory unless the
recursion option is set.  This has unintended consequences if only
files in one directory should be affected.  This patch allows
scanning the initial path parameter if it is a directory and keeps
track of the recurse depth with an extra parameter.
@harthur
Copy link
Owner

harthur commented Sep 5, 2013

Is this to kind of autocorrect not adding -r?

I don't think it would be good to add this functionality. Let's say you do replace bti bit *. That would also go into any files under directories that you had at the top level. I'd expect that command to only touch the files you have at the top level.

@markkimsal
Copy link
Author

So, if you pass just a directory to replace.js without the -r flag, nothing actually happens.

mkdir dist
cp README.md dist
node ./bin/replace.js foo bar dist/

... nothing gets changed

node ./bin/replace.js foo bar dist/ -r

...works, as wel asll:

node ./bin/replace.js foo bar dist/* 

the shell (or nomnom, i'm not sure) will expand dist/* into a list of files for the library and it doesn't have to deal with just the directory as a "paths" parameter.

When using replace.js as a library (or a grunt task), one would expect that passing a directory without special globbing expansion of wildcards would be enough for the library to iterate over files inside that directory.

After this patch is applied:

mkdir dist
cp README.md dist
node ./bin/replace.js foo bar dist
dist/README.md
 18: Replace all occurrences of "bar" with "bar" in files in the current directory:
 21: replace 'bar' 'bar' *
 27: replace 'bar' 'bar' . -r
 33: replace 'bar' 'bar' test/file1.js test/file2.js
 45: replace 'bar' 'bar' . -r --include="*.js"
 51: replace 'bar' 'bar' . -r --exclude="*.min.js,*.py"
 57: replace 'bar' 'bar' . -r --preview
 80:   regex: "bar",

This patch wouldn't actually process any files in subdirs because files in the subdir would be at depth 1 and, without the recursive option set, nothing would happen.

mkdir dist
mkdir dist/sub2
cp README.md dist
cp README.md dist/sub2

node ./bin/replace.js foo bar dist 
dist/README.md
 18: Replace all occurrences of "bar" with "bar" in files in the current directory:
 21: replace 'bar' 'bar' *
 27: replace 'bar' 'bar' . -r
 33: replace 'bar' 'bar' test/file1.js test/file2.js
 45: replace 'bar' 'bar' . -r --include="*.js"
 51: replace 'bar' 'bar' . -r --exclude="*.min.js,*.py"
 57: replace 'bar' 'bar' . -r --preview
 80:   regex: "bar",

node ./bin/replace.js foo bar dist  -r
dist/sub2/README.md
 18: Replace all occurrences of "bar" with "bar" in files in the current directory:
 21: replace 'bar' 'bar' *
 27: replace 'bar' 'bar' . -r
 33: replace 'bar' 'bar' test/file1.js test/file2.js
 45: replace 'bar' 'bar' . -r --include="*.js"
 51: replace 'bar' 'bar' . -r --exclude="*.min.js,*.py"
 57: replace 'bar' 'bar' . -r --preview
 80:   regex: "bar",

However. :(

node ./bin/replace.js foo bar dist/*
dist/README.md
 18: Replace all occurrences of "bar" with "bar" in files in the current directory:
 21: replace 'bar' 'bar' *
 27: replace 'bar' 'bar' . -r
 33: replace 'bar' 'bar' test/file1.js test/file2.js
 45: replace 'bar' 'bar' . -r --include="*.js"
 51: replace 'bar' 'bar' . -r --exclude="*.min.js,*.py"
 57: replace 'bar' 'bar' . -r --preview
 80:   regex: "bar",
dist/sub2/README.md
 18: Replace all occurrences of "bar" with "bar" in files in the current directory:
 21: replace 'bar' 'bar' *
 27: replace 'bar' 'bar' . -r
 33: replace 'bar' 'bar' test/file1.js test/file2.js
 45: replace 'bar' 'bar' . -r --include="*.js"
 51: replace 'bar' 'bar' . -r --exclude="*.min.js,*.py"
 57: replace 'bar' 'bar' . -r --preview
 80:   regex: "bar",

@harthur
Copy link
Owner

harthur commented Oct 1, 2013

Unfortunately I don't see a way to do this that is correct in all cases )=

@harthur harthur closed this Oct 1, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants