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

[Feature] Integrate language server #119

Open
10 of 12 tasks
JCWasmx86 opened this issue Mar 31, 2023 · 12 comments
Open
10 of 12 tasks

[Feature] Integrate language server #119

JCWasmx86 opened this issue Mar 31, 2023 · 12 comments

Comments

@JCWasmx86
Copy link
Contributor

JCWasmx86 commented Mar 31, 2023

I wrote a language server for meson: https://github.com/JCWasmx86/Swift-MesonLSP (GPLv3, but currently trivially changeable as I'm the only contributor)

This is a tracking issue for integration in this extension.

Requirements

  • No licensing issues
  • Works on Windows (A bit untested compared to Linux/MacOS)
  • Works on Linux
  • Works on MacOS
  • Binaries are attached to releases

Runtime dependencies

  • muon for formatting
  • wget or curl for downloading file wraps/patches
  • git/hg/svn for downloading the VCS wraps
  • git or patch for applying patches

Open questions

  • How to ship it using distro mechanisms on Linux?
  • Should muon and Swift-MesonLSP be XOR? Swift-MesonLSP uses muon for formatting and duplicates a lot of work from there. So you would have e.g. duplicate diagnostics
  • Do I have to ship licenses in the .zip files, as on Windows I ship DLLs from the swift project? (I simply do it)

I made a fork of vscode-meson for testing: https://github.com/JCWasmx86/vscode-meson

@dcbaker
Copy link
Member

dcbaker commented Mar 31, 2023

I'll write a nix expression so we can test the distro packaged aproach

@JCWasmx86
Copy link
Contributor Author

I've setup a COPR for Fedora 37, here: https://copr.fedorainfracloud.org/coprs/jcwasmx86/Swift-MesonLSP/

@JCWasmx86
Copy link
Contributor Author

An APT Repo for Ubuntu 18.04/20.04/22.04 and Debian Stable/Testing/Unstable is set up here: https://github.com/JCWasmx86/swift-mesonlsp-apt-repo

@JCWasmx86
Copy link
Contributor Author

While #123 is probably in the last 10% before merging, here is some other stuff that has to be done after merging.

  • Some sort of documentation section (Brought up in Looking for testing/feedback JCWasmx86/vscode-meson#3 (comment))
  • Auto-Updating: One approach I've seen is enumerating the latest releases on Github (=Dependency on Github and more JS dependencies). Furthermore e.g. hash values for checking the integrity (=The file is corrupted, against malicious actions the amount of work increases hundredfold) would have to be obtained via another way. Another approach - I personally prefer - would be to e.g. host a releases.json somewhere on mesonbuild.com or github pages of my user account. Looks like this:
{
  "Swift-MesonLSP": {
    "versions": [
      {
        "number": "2.2",
        "diff": "https://link/to/diff/of/previous/version",
        "downloads": {
          "win32-x64": {
            "url": "https://...",
            "hash": "12345"
          },
          "darwin-x64": {
            "url": "https://...",
            "hash": "12345"
          }
        }
      }
    ]
  }
}

Updating would require that at one person merges a PR that updates the file. While auditing it to 100% is impossible, some basic checks for malicious code could be inserted there, albeit that would require effort you can't expect from volunteers. And obviously A can't review an update for the language server by A, as otherwise this entire construct would be pointless.

I'm not sure what your threat model is, but I think it auto-updating functionality (Or even just updating using a dialog "There is an update available, do you want to install it?") needs to consider security at some point.

(Or maybe I'm thinking a bit too much about it)

@tristan957
Copy link
Contributor

Perhaps this document should just be part of the extension. I don't really see why it couldn't.

I think we should stand on the shoulders of giants in this regard. I haven't looked at how it works, but the clangd extension has pretty good update functionality.

@JCWasmx86
Copy link
Contributor Author

Yes, makes sense. But if the JSON file is in the extension, it couples the language server to the extension version. But I will implement it in some next PR (Expect it sometime in the next five weeks)

@tristan957
Copy link
Contributor

I don't really think that is a big deal if I am being honest. Maybe @dcbaker has a different opinion.

@JCWasmx86
Copy link
Contributor Author

JCWasmx86 commented Oct 3, 2023

Now post-release there are these issues left:

Somewhat easy

Complicated

  • Maybe a way to track language server crashes. No automatic uploads as (i) I have no infrastructure, (ii) it's inappropriate and (iii) a bureaucratic and legal nightmare. We can throw them just in a file. Not sure whether this is doable with the VSCode APIs.
  • Gather feedback to make the language server better. For example I thought the automatic download of subprojects was something everybody wants. Reality: Nope, don't want that
  • Auto-Update. The exact matter is already discussed with no definite results AFAIU. Add auto-updater for Swift-MesonLSP #163

@JCWasmx86
Copy link
Contributor Author

JCWasmx86 commented Oct 23, 2023

There is now server-side support for configs. It requires the configuration to be sent using the didChangeConfiguration notification with this JSON:

{
  "others": {
    "ignoreDiagnosticsFromSubprojects": true/false/["subproj1", "subproj2"], // If true, don't show any diagnostics from subprojects, if false show all diags from subprojects, if it is an array of strings, these are excluded from the diagnostics
    "neverDownloadAutomatically": true/false // Disables wrap/subproject downloads (Not implemented yet), requested by xclaesse
  },
  "linting": {
    "disableNameLinting": true/false, // Disables checks for snake_case as requested by bruchar1
    "disableAllIdLinting": true/false, // Shortcut for all the following options
    "disableCompilerIdLinting": true/false // Disable checks, whether the string compared to compiler.get_id() is known
   "disableCompilerArgumentIdLinting": true/false,
   "disableLinkerIdLinting": true/false,
   "disableCpuFamilyLinting": true/false,
   "disableOsFamilyLinting": true/false,
  }
}

@JCWasmx86
Copy link
Contributor Author

JCWasmx86 commented Feb 23, 2024

Over the past few moths, I have rewritten Swift-MesonLSP in C++ (Now called mesonlsp): JCWasmx86/mesonlsp#36 While it is not 100% finished yet, I wanted to outline the relevant changes:

Miscellaneous

  • Binary name will be renamed from Swift-MesonLSP to mesonlsp
  • No universal binaries. Those compiled on macOS 13 will be x86. macOS 14 binaries will be arm64 (Unless somebody figures out how to do universal binaries)
  • Windows support isn't done yet, as I'm missing the expertise to do that.
  • muon is statically linked into the binary => No muon binary needed anymore.

Config Options

  • linting.disableUnusedVariableCheck: This disables emission of warnings, as especially for major projects, the number is quite huge. (I did some bugfixes with the detection code, as the old code has some bugs that cause assigned variables to be registered as used, despite not being used, e.g. GStreamer with all its subprojects is at 1k+ warnings just for unused variable assignments)
  • others.defaultFormattingConfig: Default config to use for formatting, if no muon_fmt.ini exists.
  • others.removeDefaultTypesInInlayHints: Remove any|list(any)|dict(any) from inlay hints, if there are other types. This is to reduce clutter
  • others.useCustomParser: Defaults to true. The C++-Rewrite has two parsers: tree-sitter and a weird mix of the muon parser and C++ port of meson parser. The custom parser is used by default. But the tree-sitter one will remain in case of bugs (But costs performance and some advanced diagnostics)

Potential future for LSP in vscode-meson (My ideas)

  • Allow people to stay at Swift-MesonLSP (At the most recent version 3.1.3), useful e..g in case of unexpected regressions.
  • Add other language server type mesonlsp that starts with the version 4.0.0 and is the rewrite.

Thoughts of implementing something that encourages user feedback

My biggest problem is, that I basically have no feedback from the users. I asked once e.g. in the matrix channel for feedback, no responses. I'm basically in flying blind with e.g. requested features, bugs found, potential improvement ideas. I'm not sure how to solve this in a manner that is respectful to the user (==Absolutely no unwanted network connection, I'm absolutely against telemetry, and it's too much legal headache) and allows me to get user feedback.

Swift-MesonLSP 3.1.3 has 7k downlads, so let's say 6.5k users. I basically get continuous feedback from one person, I got maybe 3-4 issues with bugs/feature requests, so basically I have no idea what people want.

@siberianbot
Copy link

If I understood correctly, mesonlsp have --path parameter, which defines which file to parse as root. It will be very useful to specify path to root meson.build through VS Code settings.

I.e. my monorepo setup have several projects with different languages/build systems involved. mesonlsp looks for meson.build in root directory of workspace and I see such error messages in log:

[ WARN ] analyze::mesontree - ../src/libanalyze/mesontree.cpp:195: No meson.build file in {root directory name here}

@JCWasmx86
Copy link
Contributor Author

Theoretically a client could send a set of workspaceFolders (E.g. for the main project and one workspace folder for each subproject) to the language server, but that's currently not supported in this VSCode extension I think.

Maybe this could help temporarily: https://code.visualstudio.com/docs/editor/multi-root-workspaces But in the long-term it would be nice to have some property like paths that are a list of paths to send as workspace folder

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

4 participants