Add DjHTML package

[jordaneremieff]

Auto-indentation plugin for Django and Jinja template syntax.

[packagecontrol-bot]

Automated testing result: ERROR

Repo link: DjHTML
Results help

Packages added:
  - DjHTML

Processing package "DjHTML"
  - ERROR: The binding ['ctrl+shift+i'] unconditionally overrides a default binding
    - File: Default (Linux).sublime-keymap
  - ERROR: The binding ['ctrl+shift+i'] unconditionally overrides a default binding
    - File: Default (Windows).sublime-keymap

[braver]

🤔 maybe introduce DjHTML as a dependency so that you can use that way (and without a .no-sublime-package file)?

[packagecontrol-bot]

Automated testing result: ERROR

Repo link: DjHTML
Results help

Packages added:
  - DjHTML

Processing package "DjHTML"
  - ERROR: The binding ['ctrl+shift+i'] unconditionally overrides a default binding
    - File: Default (Linux).sublime-keymap
  - ERROR: The binding ['ctrl+shift+i'] unconditionally overrides a default binding
    - File: Default (Windows).sublime-keymap

[packagecontrol-bot]

Automated testing result: ERROR

Repo link: DjHTML
Results help

Packages added:
  - DjHTML

Processing package "DjHTML"
  - ERROR: The binding ['ctrl+shift+i'] unconditionally overrides a default binding
    - File: Default (Windows).sublime-keymap
  - ERROR: The binding ['ctrl+shift+i'] unconditionally overrides a default binding
    - File: Default (Linux).sublime-keymap

[jordaneremieff]

🤔 maybe introduce DjHTML as a dependency so that you can use that way (and without a .no-sublime-package file)?

Hi @braver, thanks for the suggestion. I am quite new to ST plugin development, and I had difficultly finding up-to-date information on how to include third-party dependencies in ST4. I would prefer not to vendor the library directly and instead include it as a dependency.

Could you point me in the right direction for how I might do this?

[braver]

There is this: https://packagecontrol.io/docs/dependencies
You can also search through the PR's here for how to add dependencies. The docs have some examples. And you can look at some packages here (like LSP for instance) on how to utilise those dependencies.

[jordaneremieff]

Thanks.

I came across this documentation initially, but it does not mention ST4 and states:

The valid list of Sublime Text versions is: st2 and st3.

I was able to dig up this issue which refers to https://github.com/wbond/package_control/projects/1, and there is also an open issue in LSP that mentions some changes are required in Package Control to support dependencies with 3.8.

I'm hesitant to pursue the dependencies solution at this point because the official support for ST4 and Python 3.8 are unclear.

[braver]

The current state of affairs as I know it is that you cannot combine py3.8 and dependencies, so you might need to stick with 3.3. There is no problem in ST4 specifically, mostly that package control is unable to handle it correctly. So, maybe the trade-off for you is to stick with / revert to 3.3 for a while until this is resolved? Not sure if that would be a problem for your package.

[jordaneremieff]

Is there a significant issue with using the vendored approach as the trade-off until the issue is resolved?

I did not discover any recommendations against including a package this way when I was researching how to create a plugin, and this step of the submission docs provides specific guidance on how to include shared libraries, which is why I included the .no-sublime-package file.

My preference is to use the current implementation until dependencies are supported for 3.8 instead of changing it twice to use the 3.3 behavior first.

[braver]

Is there a significant issue with using the vendored approach as the trade-off until the issue is resolved?

No, it's just inelegant.

Alright, let's try to add it as is. You can always improve and clean that up later.

As for your package, I haven't tested it, I'm going to assume it works as intended. There is some stuff to fine-tune though:

• You register an EventListener unconditionally (the conditions are only handled in the implementation). Would you be able to optimise that using is_applicable()?
• You also register a context menu item conditionally. Adding context menu entries is generally discouraged: the menu should be really short and it's hard for users to manage what's in there. You can make menu entries conditional so they don't take up space in contexts (e.g. syntaxes) where they don't apply. You should also consider exposing a setting that removes the menu entry. Or maybe just remove it, the primary mode to trigger commands in ST is via the command palette.
• You have a setting for which syntaxes (syntax file names) to apply to. That's a bit inelegant, because those filenames mean nothing to users. Instead you usually want to check for scopes, because all syntaxes that implement a given language tend to all share the same scope. And a scope is easier for users to find.
• You have some constants with version numbers. I'd advise against that, because you'll probably not be able to keep them in sync with your releases and actual versions. Also, you don't seem to actually use those constants.
• Your readme mentions a default keybinding that isn't there anymore.

[packagecontrol-bot]

Automated testing result: ERROR

Repo link: DjHTML
Results help

Packages added:
  - DjHTML

Processing package "DjHTML"
  - ERROR: The binding ['ctrl+shift+i'] unconditionally overrides a default binding
    - File: Default (Linux).sublime-keymap
  - ERROR: The binding ['ctrl+shift+i'] unconditionally overrides a default binding
    - File: Default (Windows).sublime-keymap

[braver]

Please respond to the feedback to continue this PR.

[jordaneremieff]

@braver I think I will just close this for now and revisit later to use the preferred dependency solution. Thanks for all the feedback you provided. I will make these changes and re-open later once the dependencies can be handled properly.