welovewordpress / SublimeHtmlTidy

Tidy, clean and prettify your HTML code with this plugin for Sublime Text 2
GNU General Public License v2.0
88 stars 15 forks source link

tidy execution broke with last update #17

Open mdsosj opened 12 years ago

mdsosj commented 12 years ago

The changes for issue #15 broke execution for me on a Windows machine:

When running tidy through Sublime Text 2 I get the following log:

Writing file /C/Users/Fr. Matthew Spencer/AppData/Roaming/Sublime Text 2/Packages/HtmlTidy/html_tidy.py with encoding UTF-8 Reloading plugin C:\Users\Fr. Matthew Spencer\AppData\Roaming\Sublime Text 2\Packages\HtmlTidy\html_tidy.py HtmlTidy: invoked on file: C:\Users\Fr. Matthew Spencer\Documents\Oblates of St. Joseph\Amazon.com\ebooks\Toschi - Joseph in the New Testament\test-doc.html HTMLTidy: using Tidy found here: C:\Users\Fr. Matthew Spencer\AppData\Roaming\Sublime Text 2\Packages\HtmlTidy\win\tidy.exe HtmlTidy: C:\Users\Fr. Matthew Spencer\AppData\Roaming\Sublime Text 2\Packages\HtmlTidy\win\tidy.exe --tab-size=2 --clean=0 --new-blocklevel-tags=section,header,footer,hgroup,nav,dialog,datalist,details,figcaption,figure,meter,output,progress --new-inline-tags=video,audio,canvas,source,embed,ruby,rt,rp,keygen,menu,command,time --new-pre-tags=article,aside,summary,mark --output-html=1 --output-xhtml=0 --output-xml=0 --show-body-only=0 --break-before-br=0 --indent=auto --indent-attributes=0 --indent-spaces=4 --wrap=0 --wrap-attributes=0 --quiet=1 --show-body-only=1 HTMLTidy experienced an error. Opening up a new file to show you.

The error file says:

'C:\Users\Fr.' is not recognized as an internal or external command, operable program or batch file.

C:\Users\Fr. Matthew Spencer\AppData\Roaming\Sublime Text 2\Packages\HtmlTidy\win\tidy.exe --tab-size 2 --clean 0 --new-blocklevel-tags section,header,footer,hgroup,nav,dialog,datalist,details,figcaption,figure,meter,output,progress --new-inline-tags video,audio,canvas,source,embed,ruby,rt,rp,keygen,menu,command,time --new-pre-tags article,aside,summary,mark --output-html 1 --output-xhtml 0 --output-xml 0 --show-body-only 0 --break-before-br 0 --indent auto --indent-attributes 0 --indent-spaces 4 --wrap 0 --wrap-attributes 0 --quiet 1 --show-body-only 1

Looks like a path with spaces in it is what might be causing the problem? When I revert to the previous changes then tidy executes correctly.

welovewordpress commented 12 years ago

Thanks @phatmatt for reporting that issue.

Yes, it's definitely caused by those spaces in the path. The whole script path should be wrapped in quotes. It actually was some time ago, but it seems this wrapping with quotes was lost along the way of the changes and code cleanup that @fitnr did.

I'm currently awaiting a response from Neil regarding his latest commits and after I have successfully merged these, I'll try to bring back those wrapping quotes. (maybe Neil could have a look too?)

fitnr commented 12 years ago

I fixed this in my recent pull. This issue wasn't exactly that it's in quotes - if the path is passed to subProcess as part of a list, it's OK.

welovewordpress commented 12 years ago

Ok I just pushed an update which doesn't use the native tidy version anymore but falls back to a webservice (which is included in the folder webservice/) currently hosted on tidy.welovewordpress.de.

Next step should probably be a setting to point this to a custom url... some might want to run their own version of the webservice.

btw: you might want to do apt-get install php5-tidy on your server to make the webservice work.

welovewordpress commented 12 years ago

@fitnr let me know if it works for you...

i needed to hack the code a bit here and there as usual... not all is pretty and on a long term, the communication with the web service should use json in both directions.

btw: why base64 encoding? because mod_security complains otherwise...

fitnr commented 12 years ago

I don't see why the local tidy.exe shouldn't be left in - for windows users without PHP, it's probably a faster option than the webservice.

As far as code structure, I suggest moving the command to call the webservice into a different function than tidy_string. Maybe split it into tidy_string_web and tidy_string_local.

welovewordpress commented 12 years ago

@fitnr It's mostly because I don't want to debug completely different versions with different feature sets and different behaviour to some parameters.

If you want to take on every single issue that gets posted here, fine. But even you probably don't want to test this plugin after each modifications on windows, osx and linux, with a bundled php version and with the native version and without both - and with custom user settings and without.

When I tried to activate the native tidy version here on osx, the whole thing just broke and I already spent wasted too much time on getting the native osx tidy and the windows tidy behave the same way... and I won't do it again, not for the sake of including software that is years old.

Speed should not be that much of an issue, regarding that you usually don't use HtmlTidy every 5 seconds... and everyone is free to care for his own working version of php - or install the webservice on localhost which works near instantly.