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

Preloads directory_watcher glob #916

Merged
merged 1 commit into from Apr 3, 2013

Conversation

Projects
None yet
4 participants
@AlexanderEkdahl
Contributor

AlexanderEkdahl commented Apr 2, 2013

This fixes all the issues I have with auto-regeneration. But seeing how this is a elusive bug feedback and testing is welcome!

What it does is simply to preload the directory before scanning it. Somehow this stops the initial repeated auto-regeneration events which may lead to an infinite loop of events.

It also always does an initial build, previously the initial build was triggered 0 or 7 times(No kidding...).

It does not require the latest(broken) version of directory_watcher.

@mattr-

This comment has been minimized.

Member

mattr- commented Apr 2, 2013

👍 from me. Thanks! ❤️

else
self.build(site, options)
end
self.build(site, options)

This comment has been minimized.

@parkr

parkr Apr 2, 2013

Member

We don't want to build if options['watch'] is set.

This comment has been minimized.

@mattr-

mattr- Apr 2, 2013

Member

Is there any harm in it though? Both methods do basically the same thing, with the exception that watch does the setting up of directory_watcher.

This comment has been minimized.

@parkr

parkr Apr 2, 2013

Member

We should try to limit redundancy as much as possible.

This comment has been minimized.

@mattr-

mattr- Apr 2, 2013

Member

Could we merge this in to fix the various bugs with automatic regeneration and then refactor later to remove the redundancy? There's already a lot of redundancy here already.

This comment has been minimized.

@parkr

parkr Apr 2, 2013

Member

Nicely illustrated with "already" there, nice one. ;-)

If we change this, we have to change the output removed as well.

This comment has been minimized.

@mattr-

mattr- Apr 2, 2013

Member

I'm a bit lost. Why do we have to change the output removed as well?

This comment has been minimized.

@parkr

parkr Apr 2, 2013

Member

The call to self.build produced that same output so it was redundant in self.watch. I wanted to ensure that the regeneration was only triggered once before serving the site, not twice. If you have a big site, you want it to only regenerate once :-)

This comment has been minimized.

@AlexanderEkdahl

AlexanderEkdahl Apr 3, 2013

Contributor

The change was intentional. It was not intended as a refactor but rather a fix for builds not completing before the server has started.

The command line output still makes sense and is less "jumpy".

@parkr parkr closed this in 91197a1 Apr 2, 2013

@AlexanderEkdahl

This comment has been minimized.

Contributor

AlexanderEkdahl commented Apr 3, 2013

directory_watcher 1.5.1 is NOT required for pre_load fix.

Also the commit you made does not fix #874

EDIT: Now auto-regeneration is broken again because a file has to be modified or no build will happen. This is frustrating...

@parkr parkr reopened this Apr 3, 2013

parkr added a commit that referenced this pull request Apr 3, 2013

@parkr parkr merged commit 5f7b9b8 into jekyll:master Apr 3, 2013

1 check passed

default The Travis build passed
Details

parkr added a commit that referenced this pull request Apr 3, 2013

Using directory_watcher 1.4.1.
1.5.1 has issues on ubuntu.
#916.

@AlexanderEkdahl AlexanderEkdahl deleted the AlexanderEkdahl:directory_pre_load branch Apr 3, 2013

@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.