Skip to content

Conversation

@iccir
Copy link
Contributor

@iccir iccir commented Dec 9, 2025

  • 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. ***
  • I use .gitattributes to exclude files from the package: images, test files, sublime-project/workspace.

My package is a "build system in reverse" - it creates a local socket and allows Sublime to receive status/issues from a continuously-running build server. Unlike LSP or build systems, Sublime is not the initiator of the build - it's just another listener.

@braver
Copy link
Collaborator

braver commented Dec 10, 2025

Very interesting!

I'm a little confused as to your implementation of settings. To begin with, the customary menu and command palette entries to open and edit the settings seem to be missing, as well a the file with default settings. Furthermore, it should not be necessary to register defaults and handle changes to settings in such a way ... although I do understand you need to re-render some views if things change. But you can easily fall back to defaults when calling sublime.Settings.get(). And if you provide a default settings file, they should also always be loaded already and merged with the user's setting, so you don't need to active manage your defaults in code anymore. Anyway... I'm probably missing something :) But there really should be defaults, menu and command palette entries, for the settings, not just a mention in the readme.

@braver braver added the feedback provided The changes and package have been seen by a reviewer label Dec 10, 2025
@iccir
Copy link
Contributor Author

iccir commented Dec 10, 2025

Thanks for the feedback! I'll investigate the customary menu and command palette entries later today.

My implementation of settings is due to type safety - I wanted to fall back to the default value if the wrong type is used (similar to how Sublime handles its internal settings) but also wanted to avoid duplicating the defaults in both a .sublime-settings file and in Python. Let me see if I can come up with a better solution for that. (It would be so much easier if sublime.Settings.get() allowed you to enforce a type).

@braver
Copy link
Collaborator

braver commented Dec 11, 2025

Ah, sure, type safety isn't a thing. Checking the type every time you fetch a value is a bit of a chore.

Perhaps you can mimic what the edit_settings command does, and instead of opening a default settings file you'd create a scratch view were you output your default settings? Anyway... as long as it's easy to interact with the settings without having to dig into the readme.

@iccir
Copy link
Contributor Author

iccir commented Dec 11, 2025

Thanks for the help!

I was able to move the settings out into a .sublime-settings file, then load it with sublime.load_resource and sublime.decode_value. That got me the default settings as a dict which I could compare to the collated sublime.Settings.

I added the menu item and command palette item as well.

iccir/BuildSock@d45b725

@braver
Copy link
Collaborator

braver commented Dec 14, 2025

Nice one. Please tag your latest version (0.1.0 points to an earlier version), and we can ship it!

@braver braver added the mergeable The channel changes are good but some action from the author is still needed label Dec 14, 2025
@iccir
Copy link
Contributor Author

iccir commented Dec 14, 2025

Done! Thanks again for all your guidance!

@braver braver merged commit 10e1d8d into sublimehq:master Dec 14, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feedback provided The changes and package have been seen by a reviewer mergeable The channel changes are good but some action from the author is still needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants