pulsar icon indicating copy to clipboard operation
pulsar copied to clipboard

Standardize language package names/descriptions

Open Spiker985 opened this issue 3 years ago • 20 comments

  • Update descriptions to have a standardized wording
  • Update language-rust-bundled to language-rust - plan to integrate a source.rust scope alongside the tree-sitter scope
  • Update language-shellscript to language-shell-script as shellscript isn't a language, but shell scripts are
  • Update langauge-coffee-script to language-coffeescript to match langauge-javascript and language-typescript
  • Bump version numbers to account for the changes in names/descriptions

Merge after #157 , there should be no merge conflicts - however, I can rectify them if they do arise

Spiker985 avatar Nov 24 '22 04:11 Spiker985

Still mid review, and this is looking great so far, and at this point good idea to bump the versions, as they have undergone a bit of change without any version change. But just do want to ask for the language-coffee-script.cson type files, do we want to rename these as well? Just so everything is cohesive, but I do want to mention, that these can be named anything according to the original Flight Manual docs, but might be worth looking at, since you may not have to do it that many times.

confused-Techie avatar Nov 24 '22 06:11 confused-Techie

Oh also, would you mind looking at the editor build? Looks like this causes some specs to fail. At least I believe the spec is calling a module by a name that no longer exists.

And last note of concern is the original docs say that the package needs to exist on NPM, although I'm not sure why. So lets hope the name change doesn't matter, either with that being outdated, or with that not effecting bundled packages. But just putting it out there as something to be aware of

confused-Techie avatar Nov 24 '22 06:11 confused-Techie

Maybe there should be a temporary changelog, like how some packages have an "unreleased" section describing all the possible features, breaking changes, etc. so that whenever a new version comes out, it's easy to say what happened.

Edit: It exists!? https://github.com/pulsar-edit/pulsar/blob/master/CHANGELOG.md

icecream17 avatar Nov 24 '22 14:11 icecream17

Oh also, would you mind looking at the editor build? Looks like this causes some specs to fail. At least I believe the spec is calling a module by a name that no longer exists.

And last note of concern is the original docs say that the package needs to exist on NPM, although I'm not sure why. So lets hope the name change doesn't matter, either with that being outdated, or with that not effecting bundled packages. But just putting it out there as something to be aware of

Something to note about the GHAs, they test against the current master, so they're not necessarily aware of the changes made inside this PR. When the GHA runs on the forked repo though, it should be fine - I just missed some things.

Specifically, I did forget to update specs outright, so I'll get those updated

Spiker985 avatar Nov 25 '22 17:11 Spiker985

And last note of concern is the original docs say that the package needs to exist on NPM, although I'm not sure why. So lets hope the name change doesn't matter, either with that being outdated, or with that not effecting bundled packages. But just putting it out there as something to be aware of

My assumption for this, is simply because they would normally be pulled from npm's repository - however, since we are explicitly telling our package.json that they are local folders, this shouldn't be an issue

Spiker985 avatar Nov 25 '22 17:11 Spiker985

(Also huge thanks to whoever bumped tree-sitter and all grammars.) Although maybe add the package renames to the changelog? (I could submit a pr for that if you want)

We probably should include this in the changelog, yeah - ideally we'd make it so the changelog automatically adds the name of merged PRs as additional lines to the changelog

Spiker985 avatar Nov 28 '22 03:11 Spiker985

Unfortunately looks like #157 did create some merge conflicts. You mentioned you'd be fine looking at them so I'll leave it as is for a bit, but feel free to ping me if any assistance is needed.

confused-Techie avatar Nov 28 '22 08:11 confused-Techie

The rename from language-rust-bundled to language-rust causes problems because a package with that name already exists! https://web.pulsar-edit.dev/packages/language-rust

sertonix avatar Nov 28 '22 11:11 sertonix

The rename from language-rust-bundled to language-rust causes problems because a package with that name already exists! web.pulsar-edit.dev/packages/language-rust

Then do we want to move the other direction, and push all the bundled package names to language-<name>-bundled? That repo is archived around Nov 8th.

Currently the original language-rust-bundled is only tree-sitter grammar and I'd actually like to learn what the difference between a tree-sitter grammar, and a source grammar is. Am I to assume that a tree-sitter grammar is based on a project root, and a source grammar is for individual file extensions? Because if that's the case, wouldn't we always want both?

Spiker985 avatar Nov 28 '22 14:11 Spiker985

Aren't the source grammars the legacy textmate ones?

Then do we want to move the other direction, and push all the bundled package names to language--bundled?

Bundled sounds a bit weird to be honest, what about marking them as core? language-core-rust etc. Keeps the important bits to the start and end of the package name.

Daeraxa avatar Nov 28 '22 16:11 Daeraxa

Bundled sounds a bit weird to be honest, what about marking them as core? language-core-rust etc. Keeps the important bits to the start and end of the package name.

I would prefer bundled instead of core because it has nothing to do with the core. The editor would run fine without it. An alternative might be language-rust-default.

Then do we want to move the other direction, and push all the bundled package names to language-<name>-bundled?

language-<name>-bundled would be a useless long name. I would accept that the name of the rust package is inconsistent. And try to support scoped packages. Then we could scope all packages with @pulsar-edit/language-rust. The rust package could then be named @pulsar-edit/language-rust.

sertonix avatar Nov 28 '22 16:11 sertonix

I would prefer bundled instead of core because it has nothing to do with the core. The editor would run fine without it. An alternative might be language-rust-default.

It is literally part of the "Core Packages" - https://github.com/pulsar-edit/pulsar/tree/master/packages#core-packages

Daeraxa avatar Nov 28 '22 17:11 Daeraxa

It is literally part of the "Core Packages" - https://github.com/pulsar-edit/pulsar/tree/master/packages#core-packages

Another thing I would rename. Core packages should be packages that the core requires to run. But they are not required.

sertonix avatar Nov 28 '22 17:11 sertonix

@pulsar-edit/core Would some of you mind chiming in on this? (Ignore the branch conflict, I'll fix those later)

Attempted to standardize the names for some of the language packages, language-rust-bundled is an outlier and there are already community packages with that name - however, that really shouldn't be an issue (currently) since core and community are still separate. However it would become an issue if we follow @Sertonix 's path of making "core" packages just like any other package

Spiker985 avatar Nov 28 '22 17:11 Spiker985

Another thing I would rename. Core packages should be packages that the core requires to run. But they are not required.

I don't think I agree with this personally. Core to me doesn't imply a technical requirement for an application to even start rather than that these packages are the minimum requirements for the standard out of the box experience as we intend it to be for it to function as a modern text editor.

That isn't to say it can't be modified and hacked about but I feel there is definitely a distinction between "core" and "essential" - the latter of which would be the minimum requirement to provide some kind of basic framework that would allow you to add your own packages onto it - at that point it might not even be a text editor until you do.

Daeraxa avatar Nov 28 '22 17:11 Daeraxa

If following Sertonix's path, maybe this name conflict could be resolved by contacting the owner of the language-rust package?

long note don't need to read: Rust (crates.io) has a similar first come first serve situation with package names (actually all package managers). The workaround there is "with agreement of old owner: add new owner; new owner removes old owner", reflecting my suggested possible solution. Name squatting solutions for Rust haven't succeeded yet, but here with this API it's theoretically much easier since it's possible to unpublish.

icecream17 avatar Nov 28 '22 22:11 icecream17

Well, ironically that package GitHub is archived as of at least the 8th.

But if I remember correctly from my reading, those grammars were made by hand

Which, we would want to presumably use a TM grammar as a base, if we were to add a new source anyway

Spiker985 avatar Nov 28 '22 22:11 Spiker985

So might not have much to add on this one. But personally I'd just suggest we find a way to name that package even if it doesn't fit our new standard 100% to avoid possible conflicts in the future. Whatever those methods may be

confused-Techie avatar Dec 10 '22 02:12 confused-Techie

So might not have much to add on this one. But personally I'd just suggest we find a way to name that package even if it doesn't fit our new standard 100% to avoid possible conflicts in the future. Whatever those methods may be

I'll just revert the naming for the rust language then, since it would conflict with the existing language package from the community.

Spiker985 avatar Dec 10 '22 14:12 Spiker985

So might not have much to add on this one. But personally I'd just suggest we find a way to name that package even if it doesn't fit our new standard 100% to avoid possible conflicts in the future. Whatever those methods may be

I'll just revert the naming for the rust language then, since it would conflict with the existing language package from the community.

That sounds like a plan to me. At the very least everything else will be consistent and obvious

confused-Techie avatar Dec 10 '22 22:12 confused-Techie

So this one has an approval, but clearly has become out of date on the master branch, is this something you'd want help with? Something you still want to implement?

confused-Techie avatar May 21 '23 02:05 confused-Techie

Deviation is too much now. Perhaps one day we'll get to do this again.

Spiker985 avatar Aug 30 '23 03:08 Spiker985