michaliskambi / elisp

Michalis' Emacs configuration. May contain some ideas/snippets that are generally useful for other people.
Other
3 stars 0 forks source link

Pascal language server #1

Closed Isopod closed 1 year ago

Isopod commented 1 year ago

Hi, I saw that you forked my LSP fork. I just wanted to address a few points you made here:

TODO: Make it aware of even LCL units? Seems like it cannot find any LCL unit, despite setting Lazarus dir.

That should not happen, I don't have that issue on any of my machines.

TODO: extremely fragile when unit on uses clause not found

I know about that, but I think that is just how CodeTools works. The same thing happens in the Lazarus IDE: Whenever there's a missing unit and you try to invoke code completion, instead of showing you candidates, the editor jumps to the erroneous line.

and its poor in finding such units. Needs

  • config to read units in Lazarus automatically?
  • read units in current project automatically.

That should work already. I suspect that there is a problem with your config if it can't find LCL units.

Ryan Joseph advantages: Passes extra FPC params from LSP initialization options. This is a clean way to pass FPC options from any LSP client.

That feature would be trivial to add, it was just not needed until now. Feel free to open a MR.

Seems a bit more active lately

That is true. My fork hasn't been very active lately for two reasons: 1. It already does 95% of what I need. 2. I'm not doing any Pascal development at the moment. However, that does not mean that development has to stop. Others are welcome to contribute. It is open source for a reason. I'd be happy to review any merge requests.

Doesn't read stuff from Lazarus config, doesn't need Lazarus config location -- it seems better to not read it if we don't need it

That may be a misunderstanding. My fork does not need the Lazarus config, either. It can use it, which is purely for convenience. But you can also specify all of the paths manually, either by sending initializationOptions or by setting environment variables.

It seems more tolerant for units missing on uses clause

It would surprise me since both forks use CodeTools under the hood. However, I'll admit I don't know much about CodeTools and the documentation is sparse to non-existent. If there is anything I could do differently, I would love to know.

Again, I'm open to contributions. Even if it is just a bug report. Please don't be shy to open an issue if you find a problem or have a suggestion.

michaliskambi commented 1 year ago

Thank you very much for this message -- the knowledge that you're active, and welcoming merge requests, is most valuable info to me.

Sorry for writing a long reply below :) I wanted to make sure I address your points. And some of my test conclusions on https://github.com/michaliskambi/elisp/tree/master/lsp have been chaotic/wrong, sorry about that too. I have improved them now after retesting things carefully, with both LSP servers (and Lazarus IDE) to have a clearer idea of the differences/similarities.

<less important information you may want to skip>

Background why I'm testing LSP servers:

  1. We are exploring the idea to "bundle" Castle Game Engine ( https://castle-engine.io/ ) download with a compiler + IDE, to make starting with the engine as easy as possible for new people (even if they don't come with Pascal knowledge, they don't know about FPC, they don't have their favourite Pascal IDE etc.). CGE downloads are already "bundled" with FPC ( https://castle-engine.io/wp/2022/10/16/cge-downloads-now-come-bundled-with-latest-stable-version-of-fpc/ ).

    The next step (planned sometime after CGE 7.0 release, at the beginning of 2023) is bundling CGE with "best Pascal IDE". What is "best Pascal IDE"? I see 2 candidates, A. Lazarus IDE or B. Visual Studio Code. The latter (VS Code) seems now a better candidate. It is just a powerful text editor, without the burden of Lazarus form designer (which we don't need in CGE, as we have own editor using CGE components for 2D and 3D games), without a multi-form Lazarus UI by default (without Lazarus Anchor Docking), well-known outside of Pascal too, with good UI, accepting command-line options to e.g. jump to file+line+column.

    But for this, we need to enable in VS Code a code completion for Pascal that is comparable with Lazarus.

    I was happy to learn that Pascal LSP servers (both your fork and Ryan Joseph) can be configured to provide this.

  2. I also come from a more selfish point of view, which is that I'm a long-time Emacs user :) It was great to finally have intelligent Pascal code completion (in contrast to "unintelligent, dictionary completion") in Emacs, thanks to both LSP servers.

So now I know we can have

</less important information you may want to skip>

TODO: Make it aware of even LCL units? Seems like it cannot find any LCL unit, despite setting Lazarus dir.

That should not happen, I don't have that issue on any of my machines.

Testcase:

So, both LSP servers are equal here. But Lazarus IDE is able to be better -- maybe because it parses LPI / LPK. I don't know the internals of CodeTools, can they be augmented with this knowledge easily.

To be frank, this is not critical to my usage. In Castle Game Engine we don't rely in LPI / LPK to compile applications, and we can use bare FPC (with CGE project definition in CastleEngineManifest.xml files).

TODO: extremely fragile when unit on uses clause not found

I know about that, but I think that is just how CodeTools works.

Confirmed and agreed. My previous statement (that it's a specific issue of your fork) was an error. Testing, confirms that both LSP servers (your's and Ryan Joseph) are equally "fragile" to missing units on uses clauses, and actually Lazarus IDE is equally "fragile". So it is a problem in CodeTools, outside of LSP servers.

<details>

What I did for testing:

</details>

Ryan Joseph advantages: Passes extra FPC params from LSP initialization options. This is a clean way to pass FPC options from any LSP client.

That feature would be trivial to add, it was just not needed until now. Feel free to open a MR.

Happy to hear you're open to this.

I'll be experimenting to see whether I prefer in long-term to pass to LSP server "FPC options" or a single "Castle Game Engine path". The latter (single CGE path) is easier to use -- it means you just set 1 path -- though it is a CGE-specific customization then.

Doesn't read stuff from Lazarus config, doesn't need Lazarus config location -- it seems better to not read it if we don't need it

That may be a misunderstanding. My fork does not need the Lazarus config, either. It can use it, which is purely for convenience. But you can also specify all of the paths manually, either by sending initializationOptions or by setting environment variables.

Thank you, and I agree after understanding this feature. It's an extra feature in your fork, not a disadvantage.

I have updated https://github.com/michaliskambi/elisp/tree/master/lsp to reflect my new knowledge.

I propose to limit this issue's scope now to resolve: why Lazarus IDE is better at finding LCL units (like Laz_AVL_Tree) than both LSP servers. I'm interested can this be improved (and it if implies the need to read LPI / LPK in LSP server, or something else).

Isopod commented 1 year ago

Thanks for your detailed response.

So, both LSP servers are equal here. But Lazarus IDE is able to be better -- maybe because it parses LPI / LPK. I don't know the internals of CodeTools, can they be augmented with this knowledge easily.

Actually, I also parse the LPI/LPK. That's the main defining feature of my fork. I needed that because I have a project consisting of multiple packages.

I'm not able to reproduce your issue with the unit not being found, for me it works: image

Probably some path is not set correctly in your case. Since those packages are part of Lazarus, you have to set LAZARUSDIR. On my system (Arch linux) that's /usr/lib/lazarus. On macOS it's /Applications/Lazarus. The contents should look like this:

[~]$ ls -lh /usr/lib/lazarus
total 54M
drwxr-xr-x 85 root root 4.0K Nov  1 23:52 components
drwxr-xr-x  2 root root 4.0K Nov  1 23:52 converter
drwxr-xr-x  4 root root 4.0K Nov  1 23:52 debugger
drwxr-xr-x  2 root root 4.0K Nov  1 23:52 designer
drwxr-xr-x  5 root root 4.0K Nov  1 23:52 doceditor
drwxr-xr-x  2 root root 4.0K Nov  1 23:52 docs
drwxr-xr-x 78 root root 4.0K Nov  1 23:52 examples
-rw-r--r--  1 root root 1.2K Oct  5 18:13 fpmake_add.inc
-rw-r--r--  1 root root  861 Oct  5 18:13 fpmake.pp
-rw-r--r--  1 root root  431 Oct  5 18:13 fpmake_proc.inc
drwxr-xr-x  4 root root  16K Nov  1 23:52 ide
drwxr-xr-x 20 root root 4.0K Nov  1 23:52 images
drwxr-xr-x  2 root root 4.0K Nov  1 23:52 languages
-rwxr-xr-x  1 root root  35M Oct  5 18:13 lazarus
-rwxr-xr-x  1 root root  12M Oct  5 18:13 lazbuild
drwxr-xr-x 12 root root 4.0K Nov  1 23:52 lcl
-rw-r--r--  1 root root  918 Oct  5 18:13 localize.bat
-rwxr-xr-x  1 root root  615 Oct  5 18:13 localize.sh
-rw-r--r--  1 root root 114K Oct  5 18:13 Makefile
-rw-r--r--  1 root root  16K Oct  5 18:13 Makefile.fpc
drwxr-xr-x  6 root root 4.0K Nov  1 23:52 packager
-rw-r--r--  1 root root 2.6K Oct  5 18:13 README.md
-rwxr-xr-x  1 root root 7.9M Oct  5 18:13 startlazarus
drwxr-xr-x 11 root root 4.0K Nov  1 23:52 test
drwxr-xr-x 10 root root 4.0K Nov  1 23:52 tools
drwxr-xr-x  3 root root 4.0K Nov  3  2021 units

The relevant packages/source files are inside the components and lcl subfolders.

For example, here is an excerpt from the log I get when editing my own pascal-language-server:

:: Using Options
  PP           = /usr/bin/fpc
  FPCDIR       = /usr/lib/fpc/src
  LAZARUSDIR   = /usr/lib/lazarus
  FPCTARGET    = linux
  FPCTARGETCPU = x86_64

:: Searching global packages
  /usr/lib/lazarus/components/*.lpk
  /usr/lib/lazarus/lcl/*.lpk
  Found 124 packages

:: Loading all packages in /home/isopod/dev/pascal-language-server
Loading /home/isopod/dev/pascal-language-server/server/pasls.lpi
  Dependency: jsonstreampkg -> /home/isopod/dev/pascal-language-server/server/deps/jsonstream/pascal/package/jsonstreampkg.lpk (hardcoded)
Loading /home/isopod/dev/pascal-language-server/server/deps/jsonstream/pascal/package/jsonstreampkg.lpk
  Dependency FCL: not found
  Dependency: LCLBase -> /usr/lib/lazarus/lcl/lclbase.lpk
Loading /usr/lib/lazarus/lcl/lclbase.lpk
  Dependency: LazUtils -> /usr/lib/lazarus/components/lazutils/lazutils.lpk
Loading /usr/lib/lazarus/components/lazutils/lazutils.lpk
  Dependency FCL: not found
  Dependency: CodeTools -> /usr/lib/lazarus/components/codetools/codetools.lpk
Loading /usr/lib/lazarus/components/codetools/codetools.lpk
  Dependency: LazUtils -> /usr/lib/lazarus/components/lazutils/lazutils.lpk
  Dependency: WebLaz -> /usr/lib/lazarus/components/fpweb/weblaz.lpk
Loading /usr/lib/lazarus/components/fpweb/weblaz.lpk
  Dependency: SQLDBLaz -> /usr/lib/lazarus/components/sqldb/sqldblaz.lpk
Loading /usr/lib/lazarus/components/sqldb/sqldblaz.lpk
  Dependency: CodeTools -> /usr/lib/lazarus/components/codetools/codetools.lpk
  Dependency: IDEIntf -> /usr/lib/lazarus/components/ideintf/ideintf.lpk
Loading /usr/lib/lazarus/components/ideintf/ideintf.lpk
  Dependency: LazControls -> /usr/lib/lazarus/components/lazcontrols/lazcontrols.lpk
Loading /usr/lib/lazarus/components/lazcontrols/lazcontrols.lpk
  Dependency: LCL -> /usr/lib/lazarus/lcl/interfaces/lcl.lpk
Loading /usr/lib/lazarus/lcl/interfaces/lcl.lpk
  Dependency: LCLBase -> /usr/lib/lazarus/lcl/lclbase.lpk
  Dependency: SynEdit -> /usr/lib/lazarus/components/synedit/synedit.lpk
Loading /usr/lib/lazarus/components/synedit/synedit.lpk
  Dependency: LCL -> /usr/lib/lazarus/lcl/interfaces/lcl.lpk
  Dependency FCL: not found
  Dependency: IDEIntf -> /usr/lib/lazarus/components/ideintf/ideintf.lpk
  Dependency FCL: not found
Loading /home/isopod/dev/pascal-language-server/server/deps/jsonstream/pascal/example/jsonecho/jsonecho.lpi
  Dependency: jsonstreampkg -> /home/isopod/dev/pascal-language-server/server/deps/jsonstream/pascal/package/jsonstreampkg.lpk (hardcoded)
Loading /home/isopod/dev/pascal-language-server/server/deps/jsonstream/pascal/example/addressbook/addressbook.lpi
  Dependency jsonstreampkg: not found
Loading /home/isopod/dev/pascal-language-server/server/deps/jsonstream/pascal/example/helloworld/helloworld.lpi
  Dependency jsonstreampkg: not found
Loading /home/isopod/dev/pascal-language-server/server/deps/jsonstream/pascal/example/recursive/recursive.lpi
  Dependency jsonstreampkg: not found
Loading /home/isopod/dev/pascal-language-server/server/deps/jsonstream/pascal/test/jsontest.lpi
  Dependency: jsonstreampkg -> /home/isopod/dev/pascal-language-server/server/deps/jsonstream/pascal/package/jsonstreampkg.lpk (hardcoded)

You should get output similar to this, otherwise there is something wrong.

michaliskambi commented 1 year ago

Got it. And it was an Emacs-specific issue, i.e. it exhibited only when Emacs was the LSP client. It worked in VS Code, as you showed.

I have submitted PR with explanation what Emacs is doing, and a proposal how to be robust in this case, in https://github.com/Isopod/pascal-language-server/pull/1 .

Sorry for not catching this fact (that it was Emacs-specific) earlier in my testing. I was sure I tested both LSP servers (your's and Ryan Joseph) with both Emacs and VS Code. I recall deliberately doing it (I even wrote "I tested above both with VS Code and Emacs integration of LSP." in my notes). Well, apparently I only tested Ryan Joseph's LSP server to confirm the problem in VS Code. Maybe I was sleepy and didn't realize that "the important difference between these 2 LSP servers is that Philip Zander's reads LPK/LPI".

It's no longer really important, but I attach the log before the fix.

lsp-log-with-emacs.txt

michaliskambi commented 1 year ago

I updated https://github.com/michaliskambi/elisp/tree/master/lsp notes and I'm extremely happy with the state of Pascal code completion in my Emacs now. After so many years of coding somewhat blindly, without intelligent code completion in my favorite editor, this is an amazing feeling :)

I'm closing this issue now.

I'll get back to you if I will find anything more to improve. I will be using this LSP server (my fork of your LSP server, which I'll try to keep close to your version sans CGE-specific modifications) in my regular daily work from now on.

  1. I will in particular look at how to improve error message on Emacs.

    I saw you already needed a non-standard solution for NeoVim ( https://github.com/Isopod/pascal-language-server/blob/master/server/utextdocument.pas#L358 ). This seems to work for Emacs too... but I wonder can it look better, as the current message looks a bit convoluted when Emacs presents it, see the screenshot. The correct error message is seen ("unit not found: Foobar") but Emacs buries it inside a Lisp processing error.

    The "correct" solution to report this, that you mention in the comment, window/showMessage, is used by https://github.com/genericptr/pascal-language-server . It works beautifully in VS Code... but Emacs seems to ignore it completely. So your solution in https://github.com/Isopod/pascal-language-server/blob/master/server/utextdocument.pas#L358 , a fake completion item, is better... but maybe it could look better in Emacs.

    Zrzut ekranu z 2022-11-10 01-34-04

  2. I am seriously thinking of bundling VS Code + Pascal LSP server with Castle Game Engine download sometime at the beginning of 2023. After this research, this looks solid, I like both LSP servers, with a slight preference towards yours now (because of LPK / LPI reading, and because the eventual errors are visible in Emacs too).

  3. I will see how much of my fork ( https://github.com/castle-engine/pascal-language-server ) I like to carry :) Ideally I'd like to submit to you everything, but I know that I'll have to carry CGE-specific modifications. In addition to current castle-pasls.ini reading, I may add ability to read CastleEngineManifest.xml, this would help with unit detection too.

  4. I'll be making some news posts about my findings of LSP servers on https://castle-engine.io/wp/ this weekend or next :)

Isopod commented 1 year ago

Regarding the error message: Maybe Emacs is getting confused because "insertText" is empty. In NeoVim, this message is just shown as a regular popup like normal completion candidates. None of these solutions are ideal.

Regarding package resolution: You may want to specify a default (or even preferred) path in your LPI/LPK. You can do this in the IDE in the project inspector (or edit the XML manually): image

This results in markup like this being inserted in the XML: https://github.com/Isopod/pascal-language-server/blob/2c09fe7b74decc98af9d4d781ce701c4761d789d/server/pasls.lpi#L50

That would help the language server find the right packages.

michaliskambi commented 1 year ago

As for error reporting: I have submitted PR https://github.com/Isopod/pascal-language-server/pull/2 :)

Below some additional Emacs-specific background info:

Testing Emacs,

  1. It was not confused by empty insertText, but by lack of isIncomplete. I guess this may be valid, as https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_completion specifies isIncomplete is required (if result is CompletionList with items sublist). Adding:

    Writer.Key('isIncomplete');
    Writer.Bool(false);

    to TextDocument_Completion reaction on ERpcError avoids Emacs reporting a scary Lisp error. But... the end result is not what we need, the completion in Emacs then just displays "No completions found" without showing the label of empty completion (which contains the error like Line 70: unit not found: Foobar that we want to see). It doesn't matter if we pass true or false as isIncomplete value.

  2. I did some reading and tested again LSP server from Ryan Joseph, https://github.com/genericptr/pascal-language-server , it uses window/showMessage (if you pass showSyntaxErrors in LSP initialization options). It turns out that Emacs supports window/showMessage. And so I implemented support for sending window/showMessage in your LSP server on https://github.com/Isopod/pascal-language-server/pull/2 .

    Sadly, window/showMessage, while in theory works, in practice is not good in Emacs in this case: the message is displayed, but only in *Messages* buffer. It blinks in the Emacs minibuffer (which is Emacs' way to report things to user) but is then quickly replaced by "No completions found" message (even if I definitely send window/showMessage after answering for completion). So, the message is displayed, but then hidden...

  3. I tried to just report LSP error, as I saw you already had API to do this.

    In Emacs, this results in the best compromise. The error message about syntax error is clearly visible (and without scary Lisp errors as before). See screenshot. https://github.com/Isopod/pascal-language-server/pull/2 enables this as an option.

Zrzut ekranu z 2022-11-14 03-33-53

michaliskambi commented 1 year ago

I'll be making some news posts about my findings of LSP servers on https://castle-engine.io/wp/ this weekend or next :)

The post about LSP servers + Castle Game Engine is live on https://castle-engine.io/wp/2022/11/19/visual-studio-code-integration-intelligent-code-completion-with-our-lsp-server-also-for-emacs-neovim-and-other-text-editors/ .

The most important part of it is a new manual page -- https://castle-engine.io/vscode . This page is an attempt to document, for users, what Visual Studio Code setup we recommend for working with Castle Game Engine. And the central part of it is using our LSP server derived from yours. We distribute the binary of LSP server precompiled in CGE download on https://castle-engine.io/download .

Isopod commented 1 year ago

Cool, thanks for the notification. If I ever set up VSCode for Pascal development, I'll probably use your manual.