-
-
Notifications
You must be signed in to change notification settings - Fork 10.1k
Cl
8000
eaner: keep_files
should only apply to the beginning of paths, not substrings with index > 0
#3849
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
Conversation
where a directory is not in keep_files, but its path contains a string in keep_files.
They were used as keywords to match files containing them in the paths.
What is the status of this? |
The example here doesn't apply to the feature. Am I misreading you here? |
First, thank you for your feedback. I am afraid you are. I think of Let me know if you are still unsure about my post. I know I am not a good writer. |
|
||
teardown do | ||
FileUtils.rm_rf(dest_dir('.git')) | ||
FileUtils.rm_rf(dest_dir('.username.github.io')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not include the preceding .
, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is correct. Thank you for pointing it out.
@shinkondo I see, thank you for your patience.
This seems to have explained it for me. You have I am 👍 on this PR. Just one comment above. |
I think what you described captures my issue, but please don't pull this request as of now. It seems to me that the situation has changed because of bd2c337. My code in pull-request would not work. Now regex generated from |
@shinkondo You're welcome to merge |
No, bd2c337 does not fix my issue. As Travis CI showed, I was wrong to say that my code would not work. It seems that the code is using absolute paths again because of bd2c337. In any case, I can fix my mistake in the test. Is there anything else I should do? I can try to squash/rebase the git log if that is preferable. |
keep_files
should only apply to the beginning of paths, not substrings with index > 0
This looks just fine to me. Thank you! @jekyllbot: merge +bug |
* origin/master: (65 commits) Update history to reflect merge of #4703 [ci skip] Update history to reflect merge of #4712 [ci skip] Highlight the test code Update history to reflect merge of #4640 [ci skip] readded "env=prod"-condition Update history to reflect merge of #3849 [ci skip] Update history to reflect merge of #4624 [ci skip] Update history to reflect merge of #4704 [ci skip] Update history to reflect merge of #4706 [ci skip] Checks for link file extension in tests Updating assets documentation Fix test teardown for cleaner. Update history to reflect merge of #4542 [ci skip] Add explanation of site variables in the example _config.yml Use double quotes in the gemfile Add test for creation of Gemfile by 'jekyll new' Add comment about github-pages Update history to reflect merge of #4533 [ci skip] Ensure Rouge closes its div/figure properly after highlighting ends. Add Site#config= which can be used to set the config ...
keep_files
could prevent more files than intended from being removed.For example, if someone is working under a folder something like
username.github.io
,keep_files: [".git", ".svn"]
matches any files underusername.github.io/_sites
, leaving the destination folder uncleaned.This would be confusing when a file is renamed while it is viewed on a browser. An user can be unaware of the fact that he is looking at outdated file (see #3847).
According to the documentation, the paths in
keep_files
should be "relative to the destination" rather than being keywords.For these reasons, I made a change so that keep_files are interpreted as paths under the destination folder.