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

rose edit capabilities document #2651

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

oliver-sanders
Copy link
Member

Partially addresses #2647

Adds configuration and metadata sections to get this document started.

There are a couple of TODOs where further research is required.

@oliver-sanders oliver-sanders added this to the 2.gui milestone Nov 28, 2022
@oliver-sanders oliver-sanders self-assigned this Nov 28, 2022
Copy link

@aenglyst aenglyst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good summary of Configuration and metadata descriptions.

Perhaps as we get a little further along we can use the descriptions from the rose metadata items to create an end-to-end mapping scheme, i.e to map fields all the way from rose metadata > jsonSchema > jsonForms. This has already been mapped for a number of metadata items, however, in the longer term it may be useful to get a complete mapping so all gaps can be identified. Where the requirement for this in the UI is essential, alternative ways to get this info into the UI can then be factored into functional specifications.

If the lack of a direct mapping is an issue for a number of fields, this should be factored into the research to be undertaken in #2643 for choice of schema and #2644 for choice of form generator.

Perhaps it is worth looking into whether there are any custom mapping solutions available to help where there is no direct mapping from metadata onto json-schema / json-form. For example this would apply to comment support, after assessing whether or not this feature is actually a requirement for Rose II.

This would apply to any other schemas chosen over these that come out of the technology choices investigations.

@oliver-sanders
Copy link
Member Author

create an end-to-end mapping scheme

That's what I'm trying to do listing the corresponding fields with each metadata item. There's probably a better way to represent this...

it may be useful to get a complete mapping so all gaps can be identified

That's the aim, just got a few gaps to fill.

Perhaps it is worth looking into whether there are any custom mapping solutions available

I think the remaining things which have no direct mapping onto JSON schema will fall into one of two camps:

  1. Things we can template into the description or work around in the metadata somehow.
    • E.G. rather than handing this in the form directly we could handle it in a metadata component associated with the form input (e.g. via an info button).
  2. Things we must handle async server-side.
    • If there are ways to extend JSON schema, great! However, some things such as fail-if/warn-if & macros require Python logic to evaluate so could not feasibly be implemented client side.

@oliver-sanders
Copy link
Member Author

Looks like $vocabulary can be used to extend the schema, this might be useful for representing Rose extensions:

http://json-schema.org/draft/2020-12/json-schema-core.html#name-example-meta-schema-with-vo

@aenglyst
Copy link

aenglyst commented Dec 5, 2022

That is good to see most of the mappings can be handled directly and the two approaches for where there is no direct mapping provide viable solutions, either client side or server side, which brings it closer to a complete end to end solution.

The use of $vocabulary is interesting and looks useful. This may require further investigation.

> **TODO:**
> Explore the view options and search features and record the key capabilities.


Copy link

@aenglyst aenglyst Dec 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A spreadsheet containing existing Rose Edit Capabilities:

Rose Edit Capabilities.xlsx

Copy link

@aenglyst aenglyst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks fine.
Some comments added and further points for discussion.

@oliver-sanders
Copy link
Member Author

LGTM.

Answered a couple of questions and filled in some TODOs in 8960c9f

Copy link

@aenglyst aenglyst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added some notes on exploring the widgets and associated capabilities

@oliver-sanders
Copy link
Member Author

oliver-sanders commented Feb 1, 2023

Assigning @wxtim who is going to try to map functionalities onto Language Server Protocol (LSP) interfaces.

A couple of questions to be resolved:

  • Can we register commands on a per-file basis with LSP?
    LSP has some functionality for registering commands which looks like it will be a good fit for Rose macros. However, the list of Rose macros available depends on the file being edited (because different apps have different macros). So we need to be able to vary the response based on the file being edited.
  • Can LSP validate files incrementally?
    Can we validate the file diff by diff rather than validating the whole file top to bottom each time something changes?

And a couple more questions about the pygls LSP implementation (which I think is the only generic LSP implementation in Python):

  • Does pygls synchronise the file being edited with the client?
    LSP has an interfaces for synchronising the file being edited between the client and the server. Can we get at this internal representation of the file from pygls or can we only see the file as it exists on the filesystem?
  • Does pygls support all of the interfaces we require?

@wxtim
Copy link
Contributor

wxtim commented Feb 9, 2023

Commands on a per-file basis.

I have as a proof of concept created a codeAction where Cylc validate is only available for a top level flow.cylc, but Cylc lint works for other files.
96b95e99-0715-4143-aaed-d5a2f0b22b94
0dc41041-9b99-4988-a030-e0c8d90f4fe1
I'm mildly concerned (but haven't fully investigated) about the possibility mentioned here that Commands have to be implemented on the client side.

Incremental Changes

Does pygls synchronise the file being edited with the client?
LSP has an interfaces for synchronising the file being edited between the client and the server. Can we get at this internal representation of the file from pygls or can we only see the file as it exists on the filesystem?

Does pygls synchronise the file being edited with the client?
LSP has an interfaces for synchronising the file being edited between the client and the server. Can we get at this internal representation of the file from pygls or can we only see the file as it exists on the filesystem?

As far as I can tell we can see the diff in the case of TEXT_DOCUMENT_DID_CHANGE event as one of the arguments the decorator function provides as params:

DidChangeTextDocumentParams(
    text_document=VersionedTextDocumentIdentifier(
        version=5, uri='file:///net/home/h02/tpilling/cylc-src/x9/flow.cylc'
    ),
    content_changes=[TextDocumentContentChangeEvent_Type1(range=6:4-6:4, text='Hello World', range_length=0)]
)

I think that this means that we could validate just the change, although we might want to memoize the rest of the validation so that the validation for unchanged lines is still returned.

It looks like the server holds a workspace object which contains representations of Document objects which contains a full representation of the doc - i.e.:

>>> ls.workspace.documents
{'file:///net/home/h02.../flow.cylc': <pygls.workspace.Doc...54201ce20>}

>>> list(ls.workspace.documents.values())[0].lines
['#!jinja2\n', '\n', '\n', '\n', '[scheduling]\n', '\tinitial cycle point = 1066\n', '    Hello World    [[graph]]\n', '        R1 = task\n', '[runtime] \n', '[meta]']
list(ls.workspace.documents.values())[0].uri
'file:///net/home/h02/tpilling/cylc-src/x9/flow.cylc'

Futher, it looks like the TEXT_DOCUMENT_DID_CHANGE method itself uses an incremental update to the internal representation

Treating different files differently

Can we register commands on a per-file basis with LSP?
LSP has some functionality for registering commands which looks like it will be a good fit for Rose macros. However, the list of Rose macros available depends on the file being edited (because different apps have different macros). So we need to be able to vary the response based on the file being edited.

I'm not sure if I've missed something - I'm not sure that you can register different commands, but what is to stop the same command doing different things depending on the file?

@wxtim
Copy link
Contributor

wxtim commented Feb 10, 2023

Recording this link - I'm not sure it's relevent, but I want to record it microsoft/language-server-protocol#642

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants