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

LSP-* versions should be based on the min/max version of LSP #97

Closed
wants to merge 5 commits into from

Conversation

predragnikolic
Copy link
Member

@predragnikolic predragnikolic commented Feb 3, 2024

Hello,

LSP is the main package that each LSP-* package imports,
so it makes sense that the version that the LSP-* packages specify, should match the min/max version of LSP,
which is:
ST 3 - 3154 -4069
ST 4 - >=4070

I have noticed the the versions of LSP-* that plugins specify are not consistent, or are sometimes even wrong("sublime_text": "3154 - 4147" - a release for ST3). So I decided to clean that up.

cc @jfcherng, @jwortmann, @rchl, @rwols

Copy link

@STReviewBot STReviewBot 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

Packages modified:
  - LSP-css
  - LSP-eslint
  - LSP-html
  - LSP-json
  - LSP-julia
  - LSP-metals
  - LSP-prisma
  - LSP-pyright
  - LSP-serenata
  - LSP-typescript
  - LSP-volar
  - LSP-yaml

Copy link

@STReviewBot STReviewBot 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

Packages modified:
  - Chialisp
  - LSP-copilot
  - LSP-cspell
  - LSP-css
  - LSP-eslint
  - LSP-html
  - LSP-json
  - LSP-julia
  - LSP-metals
  - LSP-prisma
  - LSP-pyright
  - LSP-serenata
  - LSP-typescript
  - LSP-volar
  - LSP-yaml

repository.json Outdated
"sublime_text": ">=4095",
"sublime_text": ">=4070",
Copy link
Member

Choose a reason for hiding this comment

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

This is not possible to change to 4070. I put this on purpose to 4095 because LSP-julia uses sublime.ListInputItem which was introduced with ST 4095.

The 4070 restriction for the main LSP package is mostly meaningless anyway, because I'm almost sure it uses various things from the ST API that were introduced after build 4070. Personally I also don't like the 4070- prefix for the version tags (purely for aesthetic reasons). iirc it was introduced to make it possible to add new release prefixes with increased build numbers whenever new functionality from the ST API gets used in LSP. But apparently this didn't happen in the past and also nobody really cares about it, so I guess it can just stay like this (4070) forever and in the case of any issue reports get filed in the future for the use of API incompatible with older ST builds, we could just fix those gradually with if ST_VERSION > XXXX checks in the code, if possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing that out.

I went through the LSP-* packages again
and checked if other packages use sublime.* apis introduced in later versions of ST and reverted the versions.

Copy link

@STReviewBot STReviewBot 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

Packages modified:
  - Chialisp
  - LSP-cspell
  - LSP-css
  - LSP-eslint
  - LSP-html
  - LSP-json
  - LSP-metals
  - LSP-prisma
  - LSP-pyright
  - LSP-serenata
  - LSP-typescript
  - LSP-volar
  - LSP-yaml

@predragnikolic
Copy link
Member Author

predragnikolic commented Feb 4, 2024

Also, if anyone thinks that this should be left as it was, feel free to say it.

I kind of had some urge to do it, because some versions just didn't make sense, but I'm fine with closing the PR if there is no value in it.

@rchl
Copy link
Member

rchl commented Feb 4, 2024

Also, if anyone thinks that this should be left as it was, feel free to say it.

I kind of had some urge to do it, because some versions just didn't make sense, but I'm fine with closing the PR is there is no value in it.

I think those changes have no point and potentially even make things worse because there was a ton of new stuff added and changed in 4147 vs. 4069 so it's almost guaranteed that latest LSP code would not run in 4069. The ST3 version has higher chances of working there.

@predragnikolic predragnikolic deleted the consistent-versions branch February 4, 2024 23:35
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