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

Enable sync scrolling for .. included:: files #851

Merged
merged 8 commits into from
Jul 12, 2024

Conversation

alcarney
Copy link
Member

This also improves the resolution of the scrolling as well as handling the case where the client asks to scroll to a line that doesn't have a corresponding marker. Closes #786

Closes #784

alcarney added 8 commits July 11, 2024 23:01
In order to support included files, the webview code needs access to
the source uri in addition to the line number.

Rather than even attempt to force uris to work with CSS classes, the
`SourceLocationTransform` includes a hidden div with a list of
`<span>` elements, one for each location marker.

These `<span>` elements have `data-uri` and `data-line` attributes
containing the necessary information to make sync scrolling work.
Each element has a `data-id` attribute which corresponds to a CSS
class `esbonio-marker-<id>` which links the location which its
corresponding location in the DOM
This aligns the `webview.js` file injected into HTML to the updated
source location information to add support for mutliple source files.
This aligns the language server with recent changes to enable
synchronised scrolling across multiple source files.

Additionaly, rather than send a custom `editor/scroll` notification to
the client, the server will now send a `window/showDocument` request
taking advantage of the `selection` parameter.

This *in theory* should reduce the amount of integration work required
to enable sync scrolling in new clients.
Running pytest with the `--enable-devtools` flag will enable the use
of `lsp-devtools` with the sphinx agent instance under test
Now that we have added a custom node, objects from the `sphinx_agent`
module can end up in Sphinx's `environment.pickle` file. This means in
order to be unpickled correctly Python needs to be able to do
something like `from sphinx_agent.X import Y`

However, the pytest process does not have the same Python path as the
sphinx agent process that created the `environment.pickle` file. So
when trying to load the environment in the `app` fixture, it fails and
the tests that depend on it are not running with the right data
available.

To mitigate this, the `app` fixture temporarily puts the `esbonio/`
folder onto `sys.path`, making the `sphinx_agent` module available to
the pytest process and the environment is unpickled correctly.

This doesn't necessarily feel like a great solution, but it appears to
work.

Finally, the fixture switches from using `sphinx.application.Sphinx`
to `sphinx_agent.app.Sphinx` as the `sphinx_agent` based extensions
depend on the extra `esbonio` attribute.
@alcarney alcarney merged commit e008d39 into swyddfa:develop Jul 12, 2024
13 of 14 checks passed
@alcarney alcarney deleted the preview-improvements branch July 12, 2024 20:37
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.

Improve sync scrolling resolution Extend sync scrolling to .. included:: files
1 participant