Add Explain PostgreSQL

[MGorkov]

• I'm the package's author and/or maintainer.
• I have have read the docs.
• I have tagged a release with a semver version number.
• My package repo has a description and a README describing what it's for and how to use it.
• My package doesn't add context menu entries. *
• My package doesn't add key bindings. **
• Any commands are available via the command palette.
• Preferences and keybindings (if any) are listed in the menu and the command palette, and open in split view.
• If my package is a syntax it doesn't also add a color scheme. ***
• If my package is a syntax it is named after the language it supports (without suffixes like "syntax" or "highlighting").
• I use .gitattributes to exclude files from the package: images, test files, sublime-project/workspace.

My package is Explain PostgreSQL https://github.com/MGorkov/explain-postgresql-sublime

There are no packages like it in Package Control.

It adds context menu entries, but only in sql context and their visibility is conditional.

[packagecontrol-bot]

Automated testing result: ERROR

Repo link: Explain PostgreSQL
Results help

Packages added:
  - Explain PostgreSQL

Processing package "Explain PostgreSQL"
  - ERROR: Binding is missing the keys {'command', 'keys'}
    - File: Default (OSX).sublime-keymap
    - Binding: {}
  - ERROR: Binding is missing the keys {'command', 'keys'}
    - File: Default (OSX).sublime-keymap
    - Binding: {}
  - ERROR: Binding is missing the keys {'command', 'keys'}
    - File: Default (Windows).sublime-keymap
    - Binding: {}
  - ERROR: Binding is missing the keys {'command', 'keys'}
    - File: Default (Windows).sublime-keymap
    - Binding: {}
  - ERROR: Binding is missing the keys {'command', 'keys'}
    - File: Default (Linux).sublime-keymap
    - Binding: {}
  - ERROR: Binding is missing the keys {'command', 'keys'}
    - File: Default (Linux).sublime-keymap
    - Binding: {}

[packagecontrol-bot]

Automated testing result: ERROR

Repo link: Explain PostgreSQL
Results help

Packages added:
  - Explain PostgreSQL

Processing package "Explain PostgreSQL"
  - ERROR: The binding ['super+alt+f'] unconditionally overrides a default binding
    - File: Default (OSX).sublime-keymap
  - ERROR: The binding ['super+alt+e'] unconditionally overrides a default binding
    - File: Default (OSX).sublime-keymap

[braver]

That looks useful 👍🏻 Some remarks:

• It doesn't look like the context menu items can be switched off. The context menu is prime real estate, it's usually nice if user can get some controle over that.
• Similarly the keybindings are not conditional. We usually recommend disabling bindings by default and telling users how to set them up themselves: they're really only useful if someone uses it very often and by setting them up yourself you're also more likely to remember (and you'll be able to handle clashing bindings).
• To find out if the current syntax is SQL, please do a scope check (e.g. use match_selector and look for source.sql) instead of checking the filename of the of the syntax: users might be using other syntax packages with different filenames that also provide highlighting for SQL.
• Please indicate very clearly in your README when code might leave the user's machine. It's not always clear that a service actually runs in "the cloud", and users might work in places where uploading proprietary code is a problem.
• I don't really understand why you're using a custom storage solution for settings. Settings are always immediately accessible from memory (so don't worry about performance), and live updated to reflect any changes (which you might accidentally break with your custom solutions). Just load settings and access the values you need, whenever you need them.
• I'm not really sure why you're showing a popup message, that the user has to click, instead of sending them to the analyzer thing immediately?

[MGorkov]

@braver Thanks for you comments!

Could you tell how the context menu items can be switched off ?
Plugin commands have the is_visible() so they are shown in context menu only if syntax is sql.

the rest of the changes have been completed in new release 1.0.4

• keybindings are totally disabled
• changed syntax find out method to match_selector
• added warning to README
• deleted custom storage solution
• and changed default popup to browser, so analyzer window opens immediately

[packagecontrol-bot]

Automated testing result: SUCCESS

Repo link: Explain PostgreSQL

Packages added:
  - Explain PostgreSQL

Processing package "Explain PostgreSQL"
  - All checks passed

[braver]

Sure! In the class that defines your command, you can implement an is_visible method. In that method you can check a setting, or the current scope, etc. If you want to apply a setting only to the context menu entry, you need that to be a separate command (a different class), so you can implement the is_visible method specifically for this case. But you can of course call the same command implementation.

Hope that answers your question!

[MGorkov]

@braver Thanks for you answer
This already has been implemented - all commands in the plugin have is_visible method that checks current scope.
So, the PR can be merged?

[braver]

All set 👍🏻