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 Zig Unofficial package #8958

Merged
merged 4 commits into from
Aug 16, 2024
Merged

Add Zig Unofficial package #8958

merged 4 commits into from
Aug 16, 2024

Conversation

aidenfoxivey
Copy link
Contributor

@aidenfoxivey aidenfoxivey commented Aug 9, 2024

  • 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.

This is a fork of https://github.com/ziglang/sublime-zig-language/. They haven't merged anything in 3 years from what I can tell. It currently lacks support for .zon, which is a default syntax created by zig init. If there's anything I've missed here, I'd very much welcome pointers.

Cheers,
Aiden

Signed-off-by: Aiden Fox Ivey <[email protected]>
@aidenfoxivey
Copy link
Contributor Author

Maybe I should make a semver name for it? I'm unclear whether I should just do an increment of what the original project did or whether I should start from scratch.

@FichteFoll
Copy link
Collaborator

FichteFoll commented Aug 10, 2024

Since you only made your pull request yesterday, I suggest we give the current maintainers some time to review and merge them.

That aside, we'd prefer to have only a single version of a package to centralize efforts and to reduce confusion and unnecessary choices for users, so even when the package is unmaintained, we'd rather replace it with a maintained version.

Edit: Also, yes, a semantic version tag is a requirement.

@aidenfoxivey
Copy link
Contributor Author

aidenfoxivey commented Aug 10, 2024

Since you only made your pull request yesterday, I suggest we give the current maintainers some time to review and merge them.

Absolutely! I'm in no rush at all.

That aside, we'd prefer to have only a single version of a package to centralize efforts and to reduce confusion and unnecessary choices for users, so even when the package is unmaintained, we'd rather replace it with a maintained version.

Yeah, I spoke with the creator of Zig, Andrew Kelley, about this and he said that currently the repository is unmaintained. I asked about helping maintain it and his logic was that I should try to run a maintained fork for it for ~1 year and then it could be upstreamed into the original project.

I absolutely sympathize with the desire to not have duplication - I just also don't want people using Sublime and Zig to go without some reasonable bug fixes and/or support for new features. If you've got an alternate idea here, I would totally be open to it.

Edit: Also, yes, a semantic version tag is a requirement.

Sounds good, I'll add one. I'll just increment on what the forked project did.

@aidenfoxivey
Copy link
Contributor Author

Oh hang on - I mistook which maintainers you were referring to. There are no maintainers for the main project. I'll close that PR since I am under the impression it cannot be read.

@aidenfoxivey
Copy link
Contributor Author

Looking through the other comments on the PRs from that repo, there's been no response for several years at this point. Also no maintainer.

@sigod
Copy link

sigod commented Aug 10, 2024

@aidenfoxivey Have you considered talking to someone on the Zig core team about becoming a maintainer of the sublime-zig-language package? After all, the repository is controlled by the @ziglang organization.

@aidenfoxivey
Copy link
Contributor Author

aidenfoxivey commented Aug 10, 2024

@aidenfoxivey Have you considered talking to someone on the Zig core team about becoming a maintainer of the sublime-zig-language package? After all, the repository is controlled by the @ziglang organization.

Here's the IRC logs of me talking to Andrew Kelley.

<andrewrk> aidenfoxivey: nobody, it's unmaintained
<andrewrk> oh, are you asking about all of the ziglang org? I was just talking about the zig-language repository
<aidenfoxivey> andrewrk: mostly the sublime-zig-language repository
<aidenfoxivey> is there any way I could help with getting it back together?
<aidenfoxivey> I could alternatively just do everything from my fork, but it sounds like ht be benefits in having someone sorting out the issues with the package
<andrewrk> I suggest to maintain your fork. if it's still active in one year from now you eam your work and become a maintainer in the ziglang org if you want
<aidenfoxivey> that makes sense
<aidenfoxivey> I'll take a look at the issues people seem to be filing
<aidenfoxivey> andrewrk: is it acceptable to place my fork on the sublime package registry?
<aidenfoxivey> I'd ideally like if people could use it to tell me what's wrong with it
<aidenfoxivey> but I don't want to name squat the official package
<andrewrk> I don't know why you would ask permission to do that
<andrewrk> are you going to pretend to be the official one?
<aidenfoxivey> no I don't want to pretend to be that
<aidenfoxivey> I'm honestly just really unsure
<aidenfoxivey> and I don't want to upset anyone
<andrewrk> I would only be upset if you were trying to trick people into thinking it was official one
<aidenfoxivey> that sounds reasonable
<andrewrk> the code is liberally licensed precisely so that you have the freedom to fork  whatever you want
<aidenfoxivey> I'm gonna call it Zig Unofficial
<aidenfoxivey> thanks for your guidance on this one

I would like to just maintain the original package, but I don't think that andrewrk is cool with it yet, so I want to respect his wishes.

@FichteFoll
Copy link
Collaborator

FichteFoll commented Aug 12, 2024

Hm, this is kind of an annoying situation because there is supposedly no maintainer for the repo (not even capable of merging PRs, if I understood that correctly) but at the same time they don't feel comfortable with replacing the PC entry with the fork's – or at least this it what it seems like.

In PC land, there isn't really a connotion of an "official" package. Anyone can submit a package for a language, whether it's the authors/maintainers of said language or a third party, and the only way you'd find out about that would be a sentence in the readme, a specific disclaimer in the name or description, or by checking the package's homepage/repostory and recognizing the author or organization the repository is in.

From our perspective as the maintainers of the Package Control Channel, having a fork next to the mainline Zig package provides little benefit because all the people currently using the Zig package (7k total installs, not total users) will not go out of their way to look into an alternative/fork package if they don't have significant issues with the status quo and thus your fork will most likely not be used by more than a handful of people. Any new users looking to install a package to support their work with Zig will be prompted with the option to install "Zig" or "Zig Unofficial" and in order to find out which package they'd want to install, they basically need to diff the fork and the main package on their own – unless there was a paragraph dedicated to the differences in the fork's README (which I recommend regardless).

In my opinion, it would of course be easiest if someone were to merge your pull requests so that the effort could be centralized while also having this "official" flair about it. Adding another maintainer/collaborator would of course also be fine, but I am aware that this requires a certain level of trust.

Perhaps it would be easiest if you laid out the future plans for your fork, i.e. bug fixes or additions to the package, and clarified with @andrewrk (pinging for awareness) whether this is also something they are interested in so that both sides can have expectations about the development in the near future and can choose one of the more beneficial approaches (PRs or collaborator). If that turns out to be not a viable option for now, then we (PC maintainers) will also allow the fork to exist on the channel, but please be aware that we try to avoid theses situations if possible for the aforementioned reasons.

@aidenfoxivey
Copy link
Contributor Author

I'm conflicted about whether it's a good idea to add this package myself then. I don't want to increase the complexity for the users, but I also want to fix some of the issues listed in the original repo's issue tracker. There's also some new features like syntax highlighting for .zon files or support for both types of doc comments.

I'll be totally fine if it's deemed best to avoid including my package for now.

@aidenfoxivey
Copy link
Contributor Author

My goal here is to fix problems rather than creating problems, which I fear I may have done.

@FichteFoll
Copy link
Collaborator

If there are no developments here in the next 2 weeks, I'd say we can merge this for now and improve the situation later.

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: SUCCESS

Repo link: Zig Unofficial

Packages added:
  - Zig Unofficial

Processing package "Zig Unofficial"
  - All checks passed

@FichteFoll
Copy link
Collaborator

FichteFoll commented Aug 13, 2024

Looking at your package itself:

  • You claim to support all ST versions, yet you only provide .sublime-syntax files and those are only supported by some build of ST3.
  • You have two identical Comments.tmPreferences files.
  • You'll need to update your path in the edit_settings call to open your settings because the package will be installed under the name "Zig Unofficial". The clone command in the readme should also be adjusted.
    Besides, the edit_settings command is also only available for ST3+.
  • You can remove the .tmCommand files as ST does not support them. And the info.* files in the root.
  • The package will obviously conflict with the other Zig package, so perhaps a sentence mentioning that in the readme would be appreciated.

@aidenfoxivey
Copy link
Contributor Author

Looking at your package itself:

  • You claim to support all ST versions, yet you only provide .sublime-syntax files and those are only supported by some build of ST3.
  • You have two identical Comments.tmPreferences files.
  • You'll need to update your path in the edit_settings call to open your settings because the package will be installed under the name "Zig Unofficial". The clone command in the readme should also be adjusted.
    Besides, the edit_settings command is also only available for ST3+.
  • You can remove the .tmCommand files as ST does not support them. And the info.* files in the root.
  • The package will obviously conflict with the other Zig package, so perhaps a sentence mentioning that in the readme would be appreciated.

I think those should be fixed in https://github.com/aidenfoxivey/sublime-zig-unofficial/releases/tag/v1.5.2.

@aidenfoxivey
Copy link
Contributor Author

Also fixed the claim about it supporting all versions of ST to just 3 and up.

repository/z.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: Zig Unofficial
Results help

Packages added:
  - Zig Unofficial

Processing package "Zig Unofficial"
  - WARNING: '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    - File: Syntaxes/Zon.sublime-syntax
  - WARNING: '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    - File: Syntaxes/Zig.sublime-syntax

Co-authored-by: FichteFoll <[email protected]>
@aidenfoxivey
Copy link
Contributor Author

Alright, should (possibly?) be sorted.

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: ZigUnofficial
Results help

Packages added:
  - ZigUnofficial

Processing package "ZigUnofficial"
  - WARNING: '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    - File: Syntaxes/Zon.sublime-syntax
  - WARNING: '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    - File: Syntaxes/Zig.sublime-syntax

repository/z.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: SUCCESS

Repo link: ZigUnofficial

Packages added:
  - ZigUnofficial

Processing package "ZigUnofficial"
  - All checks passed

@FichteFoll FichteFoll added the mergeable The PR is in a mergeable state but awaiting some final comments/acknowledgement from either side label Aug 16, 2024
@FichteFoll FichteFoll removed the mergeable The PR is in a mergeable state but awaiting some final comments/acknowledgement from either side label Aug 16, 2024
@FichteFoll FichteFoll merged commit 113cf28 into wbond:master Aug 16, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants