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

Typescript rewrite #22

Merged
merged 15 commits into from Nov 20, 2019
Merged

Typescript rewrite #22

merged 15 commits into from Nov 20, 2019

Conversation

swsnr
Copy link
Contributor

@swsnr swsnr commented Nov 19, 2019

A second PR, which essentially amounts to a rewrite in Typescript, which adds strong typing, linting with eslint, and uses close to no mutable state. It also prepares for reading VSCode settings asynchronously, to avoid blocking the entire shell of file system access is slow.

It also removes a few features which I don't use personally and didn't consider worth porting:

  • A proper desktop file is now required, there's no fallback to the code executable anymore.
  • The search prefix setting is gone, because I didn't use it, and I don't think it fits the design of "terms" very well.

I'm sorry for throwing such a big chance at you, but since you said in the other PR that you don't really maintain this anymore, I wanted to pick it up in a way that I can maintain it myself now.

I'm totally fine if you don't want merge it; as said, I don't really need it published and can just keep using my own fork :) However, my offer stands to take this extension over, and continue to maintain it :)

Go fully typed, and get rid of mutable things.  In particular

- turn the provider from a class into a object,
- pass all things as parameters,
- evaluate settings on the fly,
- mutate no globals other than the registered provider.

Search prefix is gone; I have no use for it, and figured it wasn't worth
porting over.
gnome-extensions pack automatically builds the schema
@swsnr swsnr changed the title Typescript Typescript rewrite Nov 19, 2019
@swsnr
Copy link
Contributor Author

swsnr commented Nov 19, 2019

Note that there are still a few todos, so this PR is preliminary. Sorry, forgot to open it as a draft.

@Jomik
Copy link
Owner

Jomik commented Nov 19, 2019

Awesome - I have added you as a collaborator on this repo. It is of course up to you whether you want to use your fork or this, but I am still keeping in touch with what is going on here, and will be happy to look through PRs.
It would be awesome if we can set up CI for pushing to GNOME extensions, but I don't think that is possible?

@swsnr
Copy link
Contributor Author

swsnr commented Nov 19, 2019

@Jomik Thanks a lot for inviting me :) I'll use this repo then, and delete my fork once this PR is merged. For the time being I'll continue making PRs, and won't push to master, so that you can keep an eye on my changes :)

I'm not sure we can push to Gnome Extensions from CI; the new gnome-extensions can only package extensions, there's no publish command. Also it requires a fairly recent version of Gnome (3.32 I guess) which is way too recent for any CI provider; they all use Ubuntu LTS which has a somewhat old Gnome version.

@Jomik
Copy link
Owner

Jomik commented Nov 19, 2019

I'm not sure we can push to Gnome Extensions from CI; the new gnome-extensions can only package extensions, there's no publish command. Also it requires a fairly recent version of Gnome (3.32 I guess) which is way too recent for any CI provider; they all use Ubuntu LTS which has a somewhat old Gnome version.

GitHub actions can run pretty much any docker container, so if there is a way to publish using something in a new gnome, then we should be able to spin up a recent GNOME instance.
Up to you if you want to spend time on it though - I can probably look at creating actions, etc., but need to know how to get a token, and such, to use for publishing :P

@swsnr
Copy link
Contributor Author

swsnr commented Nov 19, 2019

First things first, I think this PR is ready for review.

I bumped them minimum shell version to 3.34 (most recent release), because that's what I tested with on Arch. I guess it also works with earlier versions (I'd bet 3.32 at least), but this pull request uses ES6 features (e.g. arrow functions, which I'd not like to miss to be honest 😇 ) and I'm not sure how far back ES6 actually goes in terms of GJS.

GitHub actions can run pretty much any docker container, so if there is a way to publish using something in a new gnome, then we should be able to spin up a recent GNOME instance.

I don't think there is, at least not with the standard tools. Perhaps there's some 3rd party solution, but I don't think so. I guess we've got to ask the Gnome shell maintainers about this one, but if there's no solution it could be years until one exists 🤷‍♂️

I can setup a Github workflow for type checking, linting and format checking; I did that for my other Gnome shell extension, so I can just copy the file over :)

@swsnr
Copy link
Contributor Author

swsnr commented Nov 19, 2019

I asked about publishing from command line or CI in https://gitlab.gnome.org/GNOME/gnome-shell/issues/1922. Let's see what answer we'll get.

extension.ts Show resolved Hide resolved
extension.ts Outdated Show resolved Hide resolved
@swsnr swsnr self-assigned this Nov 20, 2019
@Jomik
Copy link
Owner

Jomik commented Nov 20, 2019

LGTM

@swsnr
Copy link
Contributor Author

swsnr commented Nov 20, 2019

Thanks 🙏. I’ll merge this PR then?

Concerning publishing, looks like there is really no way to publish from CLI.

@Jomik
Copy link
Owner

Jomik commented Nov 20, 2019

Yes merge :D
Can setup some automatic release thing for GitHub releases, then I can just submit that zip :)

@swsnr
Copy link
Contributor Author

swsnr commented Nov 20, 2019

Alright I’ll see if I can do that!

@swsnr swsnr merged commit 69f9705 into Jomik:master Nov 20, 2019
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.

None yet

2 participants