clangd / node-clangd

Shared features of vscode-clangd and coc-clangd
8 stars 10 forks source link

Clangd update check chokes on Apple Clangd Versioning #14

Open cpsauer opened 2 years ago

cpsauer commented 2 years ago

Hi awesome clangd folks,

I was running a test, setting things up from scratch on a new mac with the "Check Updates" feature enabled, and clangd otherwise not installed except for the Apple one in /usr/bin/clangd

I noticed that running an update check then fails with the error, presumably because it's surprised to find the Apple version's different formatting. Failed to check for clangd update: Error: Couldn't parse clangd --version output: Apple clangd version 13.0.0 (clang-1300.0.27.3) My guess at the right behavior: Disregard the apple clangd because it seems to crash consistently and immediately when invoked by the plugin.

[Running "clangd: Download language server" from the command palate still works--and is probably the best workaround for getting things going from scratch.]

Not a problem for me personally, but I bet it'll trip up some macOS users getting started, so I thought I should give a heads up.

Thanks! Chris

P.S. Tagging @sam-mccall for discussion since I think he wrote the original version in clangd/vscode-clangd#5. Sam, sorry to keep tagging you here, and hope that's okay. It's truly a super good feature!

HighCommander4 commented 2 years ago

Moved to node-clangd as that's where the relevant code lives.

https://github.com/clangd/node-clangd/commit/fd4795ca8be8d0eea953ed1e8b11e2bc98349991 is a recent change in this area which makes the version string parser accept vendor prefixes, but for Apple it still gives an error because Apple version numbers do not correspond to upstream ones.

cpsauer commented 2 years ago

Thanks, Nathan! My bad. You're quick!

sam-mccall commented 2 years ago

Current expected behavior here is that you're not prompted to install or update clangd if you're using apple clangd, because you have clangd but we don't really know its version. I think this is supposed to be silent for autoupdates, but provide some feedback if you ran the "check update" command manually.

I noticed that running an update check then fails with the error

Did you run it yourself or was the check run automatically? Where did you see the error?

Disregard the apple clangd because it seems to crash consistently and immediately when invoked by the plugin.

Just to clarify: are you saying clangd itself crashes and is unusable, or just the "check for updates" functionality doesn't work? (My understanding up to now is that apple clangd mostly works, but we have no idea which version it is)


We can also talk about changing the behavior, just want to understand the current state first.

cpsauer commented 2 years ago

Ah! Wow. Great scoping questions.

I ran the update check manually from the command palate. Didn't realize auto was disabled because of apple clangd, but since it hadn't offered to update after a reload, I triggered it manually.

On crashes: Originally, I meant both were failing, but I've realized since posting that the machine was misconfigured in a way that would make apple clangd fail. So ignore that part--sorry.

I'm realizing that those misunderstandings of mine significantly lower the expected importance. Certainly didn't intend to mislead, but mea culpa.

Reframing: It's that when an update check is requested manually and apple clangd is being used, the check fails. Error shown as notification in the lower right.

Maybe with a side-note of "user was surprised that automatic usage of apple's older clangd would suppress checks for newer versions."