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

Pascal language server #1

Closed
Isopod opened this issue Nov 6, 2022 · 8 comments
Closed

Pascal language server #1

Isopod opened this issue Nov 6, 2022 · 8 comments

Comments

@Isopod
Copy link

Isopod commented Nov 6, 2022

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 added a commit that referenced this issue Nov 7, 2022
- how "fragile" they are to missing units (resolution: both LSP
  servers, and Lazarus IDE, are equally fragile)

- how good are they at finding LCL units (resolution: both LSP
  servers behave the same; Lazarus IDE is better)

See #1
michaliskambi added a commit that referenced this issue Nov 7, 2022
@michaliskambi
Copy link
Owner

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

  • an LSP server that works great with Pascal,
  • and in particular with Castle Game Engine,
  • and I can use it with Emacs,
  • and I can recommend to "new users of CGE" to use VS Code + with the same LSP server
    ... and this makes me most happy :)

</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:

  • I opened CGE unit https://github.com/castle-engine/castle-engine/blob/master/examples/creature_behaviors/code/gamestatemenu.pas .

  • I tested my forks that add simple CGE support ( https://github.com/michaliskambi/pascal-language-server-genericptr , https://github.com/castle-engine/pascal-language-server ) -- this just results in adding a bunch of -Fuxxx, -Fixxx to FPC options. I tested also Lazarus IDE 2.2.2.

  • If I edit the TStateMenu.Start to say But and try to complete...

    begin
      inherited;
      ButtonPlay.OnClick := {$ifdef FPC}@{$endif} ClickPlay;
      ButtonQuit.OnClick := {$ifdef FPC}@{$endif} ClickQuit;
      // Hide "Quit" button on mobile/console platforms, where users don't expect such button
      ButtonQuit.Exists := ApplicationProperties.ShowUserInterfaceToQuit;
    
      But
    end;

    -> this works great. It says to me that it can complete But to ButtonPlay or ButtonQuit.

  • But if I now mess up the uses clause, adding there non-existent name...

    uses CastleApplicationProperties, CastleWindow,
      GameStatePlay, Foobar;

    -> now completion completely fails. But cannot be completed to anything. Which is a shame, it would be better IMHO if CodeTools would just ignore the missing Foobar in the uses clause.

    Testing your fork with Emacs lsp-pascal, it is explicit about the reason:

    Company: backend company-capf error "Wrong type argument: list-or-vector-p, #s(hash-table size 1 test equal rehash-size 1.5 rehash-threshold 0.8125 data ("items" [#s(hash-table size 2 test equal rehash-size 1.5 rehash-threshold 0.8125 data ("label" "Line 45: unit not found: Foobar" "insertText" ""))]))" with args (candidates Butt)
    

    Testing Ryan Joseph fork -- it is silent about the reason, but there are no completions for But. So it also fails, just without any useful message, which is actually worse.

    Testing Lazarus IDE -- it fails, clearly saying that it cannot find Foobar unit.

</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
Copy link
Author

Isopod commented Nov 7, 2022

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
Copy link
Owner

michaliskambi commented Nov 9, 2022

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 Isopod/pascal-language-server#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

  • LSP server finds the lazutils.lpk correctly (whether from Emacs or VS Code, I passed the same LAZARUSDIR), and a lot of the log seemed to make sense. There is

    Dependency: LazUtils -> /home/michalis/installed/fpclazarus/current/lazarus/components/lazutils/lazutils.lpk
    
    /home/michalis/installed/fpclazarus/current/lazarus/components/lazutils/
      UnitPath:    $(#UnitPath);/home/michalis/installed/fpclazarus/current/lazarus/components/lazutils/
      IncludePath: $(#IncPath);/home/michalis/installed/fpclazarus/current/lazarus/components/lazutils/
      SrcPath:     $(#SrcPath)
    
  • But when connected to Emacs LSP client, the LSP server didn't know RootUri. That's causing the log line

    :: Loading all packages in
    

    to suddenly end, RootUri is just empty string. Only once I compared it with VS Code, I realized it means that rootUri passed by Emacs is not understood by the LSP server.

  • I didn't analyze further why this leads to message that Laz_AVL_Tree is not found, it all works beautifully now. I.e. in Emacs I get completion after Start, when testing on the testcase provided earlier, so Emacs and VS Code both work equally.

@michaliskambi
Copy link
Owner

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
Copy link
Author

Isopod commented Nov 10, 2022

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
Copy link
Owner

As for error reporting: I have submitted PR Isopod/pascal-language-server#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 Allow to report syntax errors using LSP error and/or "window/showMessage" notification Isopod/pascal-language-server#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. Allow to report syntax errors using LSP error and/or "window/showMessage" notification Isopod/pascal-language-server#2 enables this as an option.

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

@michaliskambi
Copy link
Owner

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
Copy link
Author

Isopod commented Nov 19, 2022

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

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

2 participants