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

New auto #1734

Merged
merged 33 commits into from Jun 9, 2015
Merged

New auto #1734

merged 33 commits into from Jun 9, 2015

Conversation

@ralsina
Copy link
Member

ralsina commented May 19, 2015

This replaces livereload with a homebrew version. It seems to fix #1729 and #1757 to me, and also adds a nice thing in that it shows build failures in the web page as an alert (may be too annoying ;-)

Missing bits:

  • Cleanup
  • Support -b
  • Support -a
  • Error out on missing deps
  • Use re to inject JS instead of replace
ralsina and others added 12 commits May 19, 2015
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@Kwpolska
Copy link
Member

Kwpolska commented May 20, 2015

suggestion:

mv auto.plugin ../
mv auto.py __init__.py

Will make things a bit cleaner.

ralsina added 2 commits May 20, 2015
def inject_js(self, mimetype, data):
"""Inject livereload.js in HTML files."""
if mimetype == 'text/html':
data = re.sub('</head>', self.snippet, data, 1, re.IGNORECASE)

This comment has been minimized.

Copy link
@da2x

da2x May 21, 2015

Contributor

</body> or add <script async …>

Note that HTML5 do not guarantee any element. It requires doctype and title only.

This comment has been minimized.

Copy link
@ralsina

ralsina May 31, 2015

Author Member

I don't understand the suggestion. How would you inject it?

start_response(b'200 OK', [(b'Content-type', mimetype)])
return self.inject_js(mimetype, fd.read())
start_response(b'404 ERR', [])
return ['404 {0}'.format(uri)]

This comment has been minimized.

Copy link
@da2x

da2x May 21, 2015

Contributor

Why not inject live reload here also? User could be fiddling with their slugs or other path manipulation.

This comment has been minimized.

Copy link
@ralsina

ralsina May 31, 2015

Author Member

Good idea. Adding.

try:
ws.serve_forever()
except KeyboardInterrupt:
ws.server_close()

This comment has been minimized.

Copy link
@da2x

da2x May 21, 2015

Contributor

Log this like in the serve plugin.

'short': 'a',
'long': 'address',
'type': str,
'default': '0.0.0.0',

This comment has been minimized.

Copy link
@da2x

da2x May 21, 2015

Contributor

Time for :: by default? ;)

This comment has been minimized.

Copy link
@Kwpolska

This comment has been minimized.

Copy link
@ralsina

ralsina May 31, 2015

Author Member

I am switching to 127.0.0.1 because 0.0.0.0 acts weird in chrome: editing the path makes it a google search!

Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@ralsina ralsina changed the title [WIP] New auto New auto May 24, 2015
ralsina added 9 commits May 27, 2015
@ralsina
Copy link
Member Author

ralsina commented Jun 2, 2015

@Aeyoun @Kwpolska so ... any pending concerns with this branch?

ralsina and others added 2 commits Jun 2, 2015
import wsgiref.util

from blinker import signal
import pyinotify

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska Jun 2, 2015

Member

This works on Linux only and crashes on other platforms:

Traceback (most recent call last):
  File "C:\Python27\lib\site-packages\yapsy\PluginManager.py", line 484, in loadPlugins
    candidate_module = imp.load_module(plugin_module_name,None,candidate_filepath,("py","r",imp.PKG_DIRECTORY))
  File "C:\projects\nikola\nikola\plugins\command\auto\__init__.py", line 43, in <module>
    import pyinotify
ImportError: No module named pyinotify

You would have to find another library, like watchdog.

ralsina added a commit that referenced this pull request Jun 9, 2015
New auto
@ralsina ralsina merged commit 88c4cad into master Jun 9, 2015
3 checks passed
3 checks passed
continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ralsina ralsina deleted the new-auto branch Jun 9, 2015
@Kwpolska

This comment has been minimized.

Copy link
Member

Kwpolska commented on nikola/plugins/command/auto/__init__.py in 3d32551 Jun 11, 2015

I never got to reviewing this branch, but now I noticed this. Why are we doing shell=True with a command line here and shell=False with arguments (the recommended way) on the initial build?

This comment has been minimized.

Copy link
Member Author

ralsina replied Jun 11, 2015

No reason. Copy&Paste from somewhere if I had to guess :-)

This comment has been minimized.

Copy link
Member

Kwpolska replied Jun 11, 2015

fixed in f1e95ef

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.