Skip to content
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

Don't just ignore everything that containts the ignore paths #23

Closed
wants to merge 1 commit into from
Closed

Don't just ignore everything that containts the ignore paths #23

wants to merge 1 commit into from

Conversation

kleinfreund
Copy link

Spend my first thing I've done with Ruby with this little boy. The issue was that we were telling listen to ignore everything that contained the ignore paths (like _site). Every file that had the same string in it was not being watched by listen.

I've came to the conclusion that I needed something lightly different for files and directories, but there probably are ways without it.

  • Ignored Directories (e.g. _site): Regex should match paths that start with _site
  • Ignored files (e.g. _config.yml, .jekyll-metadata): Regex should match paths that match the relative path exactly

Phew, this took me so long. Do we need additional tests?

Ah, yes. Fixes #22 in the case I did nothing wrong. 🚀

@willnorris
Copy link

this doesn't look right. Doesn't this assume that ALL exclusions are going to be exact directories or files?Can't the excludes config setting also contain regular expressions? I think we only want to put these explicit beginning and end of string matches on the exclusions that jekyll-watch itself adds to the list. Basically everything except the result of custom_excludes(options)... so just the config files, destination directory, and .jekyll-metadata.

@kleinfreund
Copy link
Author

Can't the excludes config setting also contain regular expressions?

I have not thought about this. If this is a thing there, yes, that would screw it up. I wait for now before I make changes, because @parkr might have some helpful perspectives as well. Thank you.

@kleinfreund
Copy link
Author

@willnorris Looks quite messy, but something like this is what you had in mind, huh?

if custom_excludes(options).include?(absolute_path.to_s)
  path_to_ignore = Regexp.new(Regexp.escape(relative_path))
elsif File.directory?(relative_path)
  path_to_ignore = Regexp.new("^#{Regexp.escape(relative_path)}\/")
else
  path_to_ignore = Regexp.new("^#{Regexp.escape(relative_path)}$")
end

@kleinfreund
Copy link
Author

I think my proposed solution works around edge cases too much. If someone can propose a way of how I should work on this instead, feel free to tell me. Going to close this for now.

@parkr
Copy link
Member

parkr commented Mar 26, 2015

I think it's just a matter of ensuring the regexp, as you have above, is correct. Maybe we don't centralize the creation of the regexp, but instead have each helper method create the appropriate regexp.

@kleinfreund kleinfreund deleted the fix_listen_ignore_paths branch May 8, 2015 13:33
@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changes to files starting with _site are not being picked up
4 participants