Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add short description and example re annotation in Completions Files #121

Closed
wants to merge 7 commits into from

Conversation

axemonk
Copy link
Contributor

@axemonk axemonk commented Feb 8, 2025

First time PR to anything other than package_control_channel. Please let me know if I did something weird.

I noticed that the use of annotation isn't described in page Completions Files other than through the use of \t. I've added a short description of it and an example using it that matches the previous example.

Thanks,

@axemonk axemonk changed the title Add short description and example re annotations Add short description and example re annotation in Completions Files Feb 8, 2025
@TerminalFi
Copy link
Contributor

Appreciate the contribution. Lgtm, but I prefer to have another set of eyes just in case.

@michaelblyons
Copy link

Add a "since version ____" and I'm happy, too.

Copy link
Member

@FichteFoll FichteFoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. I have some suggestions.

@@ -82,7 +82,7 @@ is identical to the `contents`:
{ "trigger": "foo\ttest", "contents": "foobar" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to add the separate annotation form as a direct example here.

@@ -93,6 +93,12 @@ is identical to the `contents`:
slightly grayed
and does not affect the trigger itself.

Annotations can also be defined using `annotation`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in a separate annotation definition that mentions the version this was added in.

@@ -82,7 +82,7 @@ is identical to the `contents`:
{ "trigger": "foo\ttest", "contents": "foobar" }
```

**trigger**
**trigger** and **annotation**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definition should only be about trigger.

@axemonk
Copy link
Contributor Author

axemonk commented Feb 15, 2025

Not sure what the best way to preview these is. I've been using Preview in Browser through the MarkdownPreview package in Sublime. nvm---just saw the readme.

As I separated out annotations I noticed the page was also missing kind and details and began to add those, but the more I worked on this the more I realized I wasn't sure what this was supposed to provide that the official documentation doesn't already. I tried to just keep it simple and provided an example without paraphrasing anything from the official docs (exceptions being the bit about color schemes for kind and the term "kind letter").

https://www.sublimetext.com/docs/completions.html#completion-metadata

@FichteFoll
Copy link
Member

FichteFoll commented Feb 16, 2025

I wasn't sure what this was supposed to provide that the official documentation doesn't already

I don't have the full history anymore, but it is likely that the page on completions files predates the official documentation on this topic since that received an overhaul in the recent years, also leading up to ST4.

@axemonk
Copy link
Contributor Author

axemonk commented Feb 16, 2025

In that case, I'm not sure what the ground rules are here. Can I use assets from the official docs like images? Paraphrase large chunks of content? Replace an entire section with a link to the official docs?

FichteFoll pushed a commit that referenced this pull request Feb 16, 2025
Remove redundant info.

Add short description and example re `annotations`

Added completions metadata section
@FichteFoll
Copy link
Member

I made some changes on top of yours after rebasing this PR in #123, which supersedes this PR. Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants