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

[Question] Builtin AutoUpdater #11

Closed
Crashdummyy opened this issue Jul 1, 2024 · 30 comments
Closed

[Question] Builtin AutoUpdater #11

Crashdummyy opened this issue Jul 1, 2024 · 30 comments

Comments

@Crashdummyy
Copy link

Hi there

It's way too annoying to get the latest releases.

I built a public repository that will download new releases once a day.

Would you accept a PR where it'd check for updates and/or download install them directly like the old integration does with :CSInstall ?

@seblj
Copy link
Owner

seblj commented Jul 1, 2024

Interesting... Ideally, I would like to not have any lua code dealing with downloading the language server. Personally, I think I would prefer it we could get it into the mason registry.

I tried to look a bit into that myself actually a couple of days ago, but I couldn't find an easy way without doing something like creating a github repo that downloads new releases.

So what I would prefer, is that one of us tries to create a PR to mason to see if they would accept a PR. I think in the first place, we should not add it to mason-lspconfig, because that would then be included in trying to automatically setup the language server with nvim-lspconfig if a user is using the setup_handlers. I would then of course fix the path for mason download in this repo for it to just work

What do you think?

@Crashdummyy
Copy link
Author

Interesting... Ideally, I would like to not have any lua code dealing with downloading the language server. Personally, I think I would prefer it we could get it into the mason registry.

I tried to look a bit into that myself actually a couple of days ago, but I couldn't find an easy way without doing something like creating a github repo that downloads new releases.

So what I would prefer, is that one of us tries to create a PR to mason to see if they would accept a PR. I think in the first place, we should not add it to mason-lspconfig, because that would then be included in trying to automatically setup the language server with nvim-lspconfig if a user is using the setup_handlers. I would then of course fix the path for mason download in this repo for it to just work

What do you think?

Mason would be great.
I've tried to get any download at all but the azure package registry either sucks really bad or is not configured correctly.

The only reliable way to download the lsp appears to be using dotnet PackageDownload.

But that still leaves the issue at on how to obtain the latest version.
I at least think its easier to crawl the releases and provide them through github or something like that where it's easier to get the latest release

@seblj
Copy link
Owner

seblj commented Jul 1, 2024

Yes, when I tried to look into getting it to mason, I saw that there was several packages that seemed to be annoying to download, so someone created a github repo which crawls it to download (just like you have done). Then they would use that github repo to provide to mason, and from what I could understand, it would then just work with the PURL spec that mason is using.

So I think we can just point mason to your repo for downloading

@Crashdummyy
Copy link
Author

Yes, when I tried to look into getting it to mason, I saw that there was several packages that seemed to be annoying to download, so someone created a github repo which crawls it to download (just like you have done). Then they would use that github repo to provide to mason, and from what I could understand, it would then just work with the PURL spec that mason is using.

So I think we can just point mason to your repo for downloading

That'd be really great.
I am really inexperienced in using lua ( basically just starting with nvim ) but I at least plan on keeping this repository maintained at least till Microsoft provides some official way of downloading this.

But I read somewhere in a discussion or issue that this is not planned at all ( thus I created this repository )

@seblj
Copy link
Owner

seblj commented Jul 1, 2024

I can look into sending a PR to mason that downloads from your repo😄 I'll use this issue as a tracker, and can provide updates here. Thank you for creating that repo!

@Crashdummyy
Copy link
Author

I can look into sending a PR to mason that downloads from your repo😄 I'll use this issue as a tracker, and can provide updates here. Thank you for creating that repo!

Thanks.
That'd be amazing.

How'd you go about providing weird hacks in the default configuration?

Like semantics wont work as of now so you have to monkey patch the request a bit.

function M.monkeyPatchSemanticTokens(client)
  local utils = require("base.utils")
  -- make sure this happens once per client, not per buffer
  if not client.is_hacked then
    client.is_hacked = true
  
    -- let the runtime know the server can do semanticTokens/full now
    if client.server_capabilities.semanticTokensProvider then
      client.server_capabilities = vim.tbl_deep_extend("force", client.server_capabilities, {
        semanticTokensProvider = {
          full = true,
        }, 
      })
    end

    utils.notify("patching the semantic tokens for roslyn")
    -- monkey patch the request proxy
    local request_inner = client.request
    client.request = function(method, params, handler)
      if method ~= vim.lsp.protocol.Methods.textDocument_semanticTokens_full then
        return request_inner(method, params, handler)
      end
    
      local function find_buf_by_uri(search_uri)
        local bufs = vim.api.nvim_list_bufs()
        for _, buf in ipairs(bufs) do
          local name = vim.api.nvim_buf_get_name(buf)
          local uri = "file://" .. name
          if uri == search_uri then
            return buf
          end
        end
      end

      local doc_uri = params.textDocument.uri
    
      local target_bufnr = find_buf_by_uri(doc_uri)
      local line_count = vim.api.nvim_buf_line_count(target_bufnr)
      local last_line = vim.api.nvim_buf_get_lines(target_bufnr, line_count - 1, line_count, true)[1]

      return request_inner("textDocument/semanticTokens/range", {
        textDocument = params.textDocument,
        range = {
          ["start"] = {
            line = 0,
            character = 0,
          },
          ["end"] = {
            line = line_count - 1,
            character = string.len(last_line) - 1,
          },
        },
      }, handler)
    end
  end
end

@seblj
Copy link
Owner

seblj commented Jul 1, 2024

How'd you go about providing weird hacks in the default configuration?

What do you mean? If I am open to including this hack or?

I am not so familiar with semantic tokens and the specs for it. Could you explain a bit about why it is needed to patch it?
I am open to include something like this if I can understand it a bit more and know why we do it😄

BTW: Already started looking into adding it to mason, and it is going great as of now! Could you do me a favour though? Could you also crawl for osx-x64 and win-arm64? It seems to be normal to also provide those.

Other than that, I think I have something working locally now

@SimonSchwendele
Copy link

How'd you go about providing weird hacks in the default configuration?

What do you mean? If I am open to including this hack or?

I am not so familiar with semantic tokens and the specs for it. Could you explain a bit about why it is needed to patch it? I am open to include something like this if I can understand it a bit more and know why we do it😄

BTW: Already started looking into adding it to mason, and it is going great as of now! Could you do me a favour though? Could you also crawl for osx-x64 and win-arm64? It seems to be normal to also provide those.

Other than that, I think I have something working locally now

Oh no problem I'll add those in a sec.

The highlight doesn't work as Roslyn doesnt implement textDocuments/semantic tokens/full.
This just patches range to be exposed as full.

neovim/neovim#26500

@seblj
Copy link
Owner

seblj commented Jul 1, 2024

Ahaa, I understand! I can look into adding that hack as a built in support :)

@Crashdummyy
Copy link
Author

Ahaa, I understand! I can look into adding that hack as a built in support :)

I added the remaining versions

@seblj
Copy link
Owner

seblj commented Jul 1, 2024

Thank you! I opened this: mason-org/mason-registry#6330 so lets hope it gets accepted and is merged soon😄

@seblj
Copy link
Owner

seblj commented Jul 1, 2024

I started looking into adding support for semantic tokens without you needing to have the snippet you sent in your config. It seems to work okay, but I need to think a little bit more about it and how I should do it, because it seems to be a bit buggy sometimes🤔
image

At first it seemed to be like that until I started to edit the text or do :e, but now all of a sudden it seems to correct itself after a second or so.

I need to play with it a little bit to see how it behaves before I decide if I have to add a config for this or not, or if it should be enabled by default or not

@Crashdummyy
Copy link
Author

I started looking into adding support for semantic tokens without you needing to have the snippet you sent in your config. It seems to work okay, but I need to think a little bit more about it and how I should do it, because it seems to be a bit buggy sometimes🤔 image

At first it seemed to be like that until I started to edit the text or do :e, but now all of a sudden it seems to correct itself after a second or so.

I need to play with it a little bit to see how it behaves before I decide if I have to add a config for this or not, or if it should be enabled by default or not

I discovered similiarily weird behaviors as of today.
Sometimes stuff begins loading slowly,
I might be forced to toggle syntax and nvim-treesitter on and off sometimes.

I still cant figure out the reason behind that

@ADIX7
Copy link

ADIX7 commented Jul 6, 2024

Just a note, with the patch above (added to on_attach) breaks https://github.com/Wansmer/symbol-usage.nvim

@ADIX7
Copy link

ADIX7 commented Jul 16, 2024

I managed to fix this. There is a 4th parameter for client.request, buffer number, that also has to be passed.
This is the updated snippet.

if not client.is_hacked_roslyn then
    client.is_hacked_roslyn = true

    -- let the runtime know the server can do semanticTokens/full now
    if client.server_capabilities.semanticTokensProvider then
        client.server_capabilities = vim.tbl_deep_extend("force", client.server_capabilities, {
            semanticTokensProvider = {
                full = true,
            },
        })
    end

    -- -- monkey patch the request proxy
    local request_inner = client.request
    client.request = function(method, params, handler, req_bufnr)
        if method ~= vim.lsp.protocol.Methods.textDocument_semanticTokens_full then
            return request_inner(method, params, handler, req_bufnr)
        end

        local function find_buf_by_uri(search_uri)
            local bufs = vim.api.nvim_list_bufs()
            for _, buf in ipairs(bufs) do
                local name = vim.api.nvim_buf_get_name(buf)
                local uri = "file://" .. name
                if uri == search_uri then
                    return buf
                end
            end
        end

        local doc_uri = params.textDocument.uri

        local target_bufnr = find_buf_by_uri(doc_uri)
        local line_count = vim.api.nvim_buf_line_count(target_bufnr)
        local last_line = vim.api.nvim_buf_get_lines(target_bufnr, line_count - 1, line_count,
            true)[1]

        return request_inner("textDocument/semanticTokens/range", {
                textDocument = params.textDocument,
                range = {
                    ["start"] = {
                        line = 0,
                        character = 0,
                    },
                    ["end"] = {
                        line = line_count - 1,
                        character = string.len(last_line) - 1,
                    },
                },
            },
            handler,
            req_bufnr
        )
    end
end

@Syndim
Copy link

Syndim commented Aug 17, 2024

While waiting for the PR to be merged in mason-registry, I created a custom registry that can be used to install roslyn from mason. Just add the registries config to mason's setup and you will be able to install roslyn:

require('mason').setup({
        registries = {
            'github:mason-org/mason-registry',
            'github:syndim/mason-registry'
        },
    })

@Crashdummyy
Copy link
Author

While waiting for the PR to be merged in mason-registry, I created a custom registry that can be used to install roslyn from mason. Just add the registries config to mason's setup and you will be able to install roslyn:

require('mason').setup({
        registries = {
            'github:mason-org/mason-registry',
            'github:syndim/mason-registry'
        },
    })

That's cool :)
I currently just retain the last 15 releases which ammounts to 3-4 weeks regarding their current velocity.
Do I need to explicitly retain the pinned version in your registry or do you plan to update every 2 weeks or sth. like this anyways ?

@Syndim
Copy link

Syndim commented Aug 17, 2024

While waiting for the PR to be merged in mason-registry, I created a custom registry that can be used to install roslyn from mason. Just add the registries config to mason's setup and you will be able to install roslyn:

require('mason').setup({
        registries = {
            'github:mason-org/mason-registry',
            'github:syndim/mason-registry'
        },
    })

That's cool :) I currently just retain the last 15 releases which ammounts to 3-4 weeks regarding their current velocity. Do I need to explicitly retain the pinned version in your registry or do you plan to update every 2 weeks or sth. like this anyways ?

I setup a bot to automatically update the package version. Currently it's sending PRs to the repo, will try to let the bot commit to the repo directly so no action will be needed.

@molostovvs
Copy link

@Syndim
Hi! it seems your feed is out of sync with the microsoft feed - your feed hasn't had an update in 2 weeks. Do you know about this?

@Crashdummyy
Copy link
Author

@Syndim Hi! it seems your feed is out of sync with the microsoft feed - your feed hasn't had an update in 2 weeks. Do you know about this?

I guess renovate isn't configured correctly.
Roslyn bumped from 4.12 to 4.13.

@kmoschcau
Copy link

kmoschcau commented Oct 15, 2024

Hi, I have a small patch to suggest for the semantic tokens. I can't clone or edit the wiki, so I'm doing it here. You only have to add some vim.fs.normalize calls to find_buf_by_uri to make it work on Windows file systems and with relative paths.

    local function find_buf_by_uri(search_uri)
      local bufs = vim.api.nvim_list_bufs()
      local normalized_search_uri = vim.fs.normalize(search_uri)
      for _, buf in ipairs(bufs) do
        local name = vim.api.nvim_buf_get_name(buf)
        local uri = vim.fs.normalize("file://" .. name)
        if uri == normalized_search_uri then
          return buf
        end
      end
    end

Side note: The semantic highlighting seems to do a large amount of requests, noticeably slowing down everything after inserting some characters.

@Syndim
Copy link

Syndim commented Oct 15, 2024

@Syndim Hi! it seems your feed is out of sync with the microsoft feed - your feed hasn't had an update in 2 weeks. Do you know about this?

@Syndim Hi! it seems your feed is out of sync with the microsoft feed - your feed hasn't had an update in 2 weeks. Do you know about this?

I guess renovate isn't configured correctly. Roslyn bumped from 4.12 to 4.13.

Fixed

@seblj
Copy link
Owner

seblj commented Oct 15, 2024

Hi, I have a small patch to suggest for the semantic tokens. I can't clone or edit the wiki, so I'm doing it here. You only have to add some vim.fs.normalize calls to find_buf_by_uri to make it work on Windows file systems and with relative paths.

    local function find_buf_by_uri(search_uri)

      local bufs = vim.api.nvim_list_bufs()

      local normalized_search_uri = vim.fs.normalize(search_uri)

      for _, buf in ipairs(bufs) do

        local name = vim.api.nvim_buf_get_name(buf)

        local uri = vim.fs.normalize("file://" .. name)

        if uri == normalized_search_uri then

          return buf

        end

      end

    end

Side note: The semantic highlighting seems to do a large amount of requests, noticeably slowing down everything after inserting some characters.

I'll try to update the tips and tricks

@nagaohiroki
Copy link

Hi, I have a small patch to suggest for the semantic tokens. I can't clone or edit the wiki, so I'm doing it here. You only have to add some vim.fs.normalize calls to find_buf_by_uri to make it work on Windows file systems and with relative paths.

If you change find_buf_by_uri to vim.uri_to_bufnr, it will work on Windows too. I think it will be simpler.

local target_bufnr = find_buf_by_uri(params.textDocument.uri)

local target_bufnr = vim.uri_to_bufnr(params.textDocument.uri)

@seblj
Copy link
Owner

seblj commented Nov 10, 2024

So a little update:

I have used a shell script for a while to automatically update roslyn for me, and that works pretty good for me at least. However, I understand the wish to get it into mason. It seems like it will not be merged into mason shortly, and as a workaround for this, I am thinking about including something like this: #80

It should automatically set up mason with the additional registry from @Syndim. What do you think about this approach?

@Crashdummyy
Copy link
Author

So a little update:

I have used a shell script for a while to automatically update roslyn for me, and that works pretty good for me at least. However, I understand the wish to get it into mason. It seems like it will not be merged into mason shortly, and as a workaround for this, I am thinking about including something like this: #80

It should automatically set up mason with the additional registry from @Syndim. What do you think about this approach?

Yeah...
Doesnt feel like its going to be merged any time soon.
Besides the frequent updates of roslyn will make this even harder.
I forked syndims repository and exapnded it by rzls already which got the same issues.
The rzls updates arent that frequent tho.

I think keeping it in some way inside your plugin is way better than waiting for mason

@seblj
Copy link
Owner

seblj commented Nov 10, 2024

Okay, I think it makes sense to use your fork then as you then have both the repo that automatically gets updates, and also the registry then. Would you be okay with me using that in the PR i linked?

@Crashdummyy
Copy link
Author

Okay, I think it makes sense to use your fork then as you then have both the repo that automatically gets updates, and also the registry then. Would you be okay with me using that in the PR i linked?

Sure I really appreciate this.
I plan to maintain this until theres a better solution anyway

@seblj
Copy link
Owner

seblj commented Nov 10, 2024

Perfect!

@seblj
Copy link
Owner

seblj commented Nov 10, 2024

So I just merged a PR which automatically sets up the registry for mason if mason is installed. While I wished that mason could include it in their original registry, I think this should for the moment at least make it a lot easier for user, and is now also kind of a "builtin autoupdater". So I think therefore I will close this issue as completed for now!

Thanks for maintaining the autoupdater and mason registry! Really appreciate it!

@seblj seblj closed this as completed Nov 10, 2024
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

No branches or pull requests

8 participants