Funny behavior with pre-commit and Windows

[spapas]

Hello, I was using the following snippet for pre-commit:

• repo: https://github.com/rtts/djhtml
  rev: v1.5.2
  hooks:
  • id: djhtml
    entry: djhtml -i -t 2
  • id: djcss
    entry: djcss -i -t 2
  • id: djjs
    entry: djjs -i -t 2

This resulted in removing the identation of the css and js files when pre-commit was run (similar to this issue #66).

When I tried running djcss file.css or djjs file.js it worked fine but when it was run through pre-commit it removed the identation!

After some tests, I changed my hooks to :

      - id: djhtml
        entry: djhtml.exe -i -t 2
      - id: djcss
        entry: djcss.exe -i -t 2
      - id: djcss
        entry: djjs.exe -i -t 2

and now it works! My understanding is that when the djcss/djjs scripts are run through pre-commit it can't match their filenames and falls back upon the djhtml which seems to be the default (see here https://github.com/rtts/djhtml/blob/main/djhtml/__main__.py#L34) ?

One possible solution would be to change the corresponding lines (

djhtml/djhtml/__main__.py

Line 34 in 295a54c

if sys.argv[0].endswith("djtxt"):

etc) to something like if sys.argv[0].endswith("djtxt") or sys.argv[0].endswith("djtxt.exe"):. However I can't easily test it since I don't know how to generate these .exe files and I understand that if you don't use windows it also wouldn't be easy for you to test...

Another simpler solution would be to add a hint for Windows devs in the README.md (i.e windows users should always add the enrty: djjs.exe -i and djcss.exe lines)

[spapas]

Also, after a quick peek at #66 it seems that the OP was also using Windows. @GitRon can you try the proposed solution here?

[JaapJoris]

Welcome @spapas, and thank you for finally figuring out the cause of #66!

Your solution by special-casing Windows executable filenames would work, but what would also work is to remove the -i flag altogether so that a plain invocation of djhtml.exe would modify files in place (instead of write to stdout).

I don't think many people are using djhtml without the -i flag anyway, so it could be a good idea to make that the default. What do you think?

[spapas]

Hello @JaapJoris the approach you propose is fine for me, however I'd really recommend to allow a flag for outputing to stdout. This would be important for testing purposes.

Also please notice that if you remove the -i flag it may break some people's workflows.

[GitRon]

@spapas I think @JaapJoris suggested to make it a default and not remove it?

[spapas]

@GitRon how would the -i flag be the default without removing it ?

[GitRon]

Oh, sorry, misread it. But the issue could be solved with publishing a major release, right? So people get the indication that something happened.

[GitRon]

@JaapJoris Awesome! Any plans for when the new release will come out?

[JaapJoris]

@GitRon Very soon!

[JaapJoris]

@GitRon @spapas DjHTML version 2.0.0 has just been released! I don't have access to a Window machine, but I have very good hopes that this issue is now solved. Please confirm that it is, or reopen it if it isn't 🙏

[JaapJoris]

Finally, note there is actually a reason for the default mode being DjHTML: besides using the command djhtml, djcss, etc., you can also call DjHTML using python -m:

python -m djhtml file.html

Since the module is called djhtml, it makes sense for the mode to be DjHTML when called like this, despite sys.argv[0] actually being __main__.py in that case.

[spapas]

Hello @JaapJoris seems to be working fine now on my windows system! Thank you