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

Add some example servers #440

Merged
merged 8 commits into from
Apr 3, 2024
Merged

Conversation

alcarney
Copy link
Collaborator

I'm concious of the fact #418 is a rather large PR!

I've also realised that there's a growing amount of work in #418 that is not directly dependant on the new architecture.
So, this PR is a first pass at breaking some of that work out into a separate PR which we can merge now.

As part of refactoring the tests under tests/lsp to be end-to-end tests, I have been introducing small example servers which not only help with the tests, but which should help anyone new to pygls/lsp get to grips with how things work.
This PR adds the following example servers:

Filename Works With Description
code_lens.py sums.txt Evaluate sums via a code lens
colors.py colors.txt Provides a visual representation of color values and even a color picker in supported clients
goto.py code.txt Implements the various "Goto X" requests in the specification for a dummy programming language
hover.py dates.txt Opens a popup showing the date underneath the cursor in multiple formats
publish_diagnostics.py sums.txt Use "push-model" diagnostics to highlight missing or incorrect answers
pull_diagnostics.py sums.txt Use "pull-model" diagnostics to highlight missing or incorrect answers

Side note: I'm thinking we should start moving away from json_server.py as our flagship example for two reasons.

  1. Small focused examples are probably easier to parse than one big one.
  2. Editors like VSCode have a lot of built-in support for JSON - it might not be obvious to tell which features are from pygls and which are from the editor.

Code review checklist (for code reviewer to complete)

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • Commit messages are meaningful (see this for details)
  • Tests have been included and/or updated, as appropriate
  • Docstrings have been included and/or updated, as appropriate
  • Standalone docs have been updated accordingly

@tombh
Copy link
Collaborator

tombh commented Mar 17, 2024

This is epic, so many useful examples and tests 🥰 I'll take a good long look soon.

@tombh
Copy link
Collaborator

tombh commented Mar 28, 2024

Sorry for the delay, I've had a good look and think about this now. Again, really epic work here. This is so much more than just adding great new tests, it's self-documenting and generous to newcomers and experts alike.

Also, as always, thank you so much for the self-contained commits, they really make understanding the changes here more digestible.

I'd be happy to merge this as is! But here are some comments, they're very minor, so unless they resonate, don't worry about them:

  • Could the example server's README be linked to from the main README? And what about linking each example server from the bottom of https://pygls.readthedocs.io/en/latest/pages/getting_started.html#quick-start?
  • You have added some comments to some of the example servers, and generally I roughly understand what each one does. But I'm wondering if a brief description could be added to each explaining what the in-editor experience of using the server might be like? For example, for the hover server: "When the cursor is placed over a date, you should see a small popup describing the recognised Python strftime formatting for the date"
  • The following seems to be a common pattern:
      client, response = await get_client_for("...py")
      yield client, response
    
      await client.shutdown_async(None)
      client.exit(None)
    
      await client.stop()
    To what extent can it be refactored?
  • Could assert test_uri is not None live in the uri_for function itself?

Dwi'n dysgu am englyn yn fy dosbarthiau cymraeg ddiweddar. Yr PR yma yn atgoffa o y llinell glasurol:

Campwaith dewin hynod

🪄

@alcarney
Copy link
Collaborator Author

Could assert test_uri is not None live in the uri_for function itself?

Yes, good spot

The following seems to be a common pattern: To what extent can it be refactored?

Perhaps get_client_for can handle it and the fixture just becomes yield from get_client_for(...), I'll have a play and see if it will work

This commit refactors the infrastructure for writing end-to-end tests.

There are now three fixtures `get_client_for` and `path_for` and
`uri_for`, as the names suggest these are functions for obtaining a
`LanguageClient` connected to a server in `examples/servers` and
filepaths or URIs for files in `examples/servers/workspace`.

This also adds a custom cli flag to our test suite - `--lsp-runtime`
for choosing the runtime that the end-to-end tests are run against.

Currently, the only supported runtime is `cpython` but this paves the
way for other runtimes in the future (such as Pyodide or WASI)
@alcarney
Copy link
Collaborator Author

alcarney commented Apr 3, 2024

Turns out you can't yield from an async generator, but an async for loop seems to work, so the fixtures now have the form

@pytest_asyncio.fixture()
async def json_server(get_client_for):
    async for result in get_client_for("json_server.py"):
        yield result

And what about linking each example server from the bottom of https://pygls.readthedocs.io/en/latest/pages/getting_started.html#quick-start?

I'll probably handle this in a separate PR - perhaps #427

@tombh
Copy link
Collaborator

tombh commented Apr 3, 2024

Great 😊

@alcarney alcarney merged commit 850b86c into openlawlibrary:main Apr 3, 2024
16 checks passed
@alcarney alcarney deleted the example-servers branch April 3, 2024 17:29
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

Successfully merging this pull request may close these issues.

2 participants