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

Better code completion proposal for name/key of patterned properties and array elements #227

Open
tfesenko opened this issue Oct 18, 2016 · 7 comments

Comments

@tfesenko
Copy link
Member

tfesenko commented Oct 18, 2016

We use the _key_ as a code assist proposal for the "no proposal" case, usually it means property name, or some other named element, represented as additionalProperties or patternProperties in the schema:
screen shot 2016-10-17 at 8 47 55 pm
and
screen shot 2016-10-17 at 8 48 04 pm

It can be easily changed as it's defined in com.reprezen.swagedit.assist.SwaggerProposalProvider.createObjectProposals(ObjectTypeDefinition, AbstractNode, String):

if (proposals.isEmpty()) {
    proposals.add(new Proposal("_key_" + ":", "_key_", null, null));
}

Note that we can change the label, the replacement text, or both.

@tedepstein , we wonder if you have ideas how to improve it, which labels/replacement use in this case.

@tedepstein
Copy link
Collaborator

tedepstein commented Oct 18, 2016

@tfesenko , can the suggestion be context-sensitive, so we have something like (property name) in the context shown, and some other appropriate label in other contexts?

@tedepstein tedepstein changed the title Better code completion proposal for property names Better code completion proposal for name/key of additionalProperties Sep 22, 2017
@tedepstein
Copy link
Collaborator

tedepstein commented Sep 22, 2017

@tfesenko , @ghillairet , I'm placing high priority on this one, though I don't necessarily think we should jump into implementation. There are some things we should probably do first, just to make sure that OpenAPI v3 support is complete.

But this is a weak point in the usability of KaiZen editor right now. It's also closely related to #70, and maybe these two issues should be merged. Generally, the problem is with user-specified key names, represented in the schema as additionalProperties or patternProperties. With additionalProperties, we propose _key_. With pattern properties, we propose the pattern, as it's specified in the schema. In both cases, it's confusing to the user, and if the user selects the proposal, it puts it in verbatim, with no hint to the user that they should put in a proper name, and this immediately puts the editor in an error state.

Basic Usability Improvements

So far, we've suggested the following improvements:

  • Make the proposal name more descriptive of what's expected. For example schema_name instead of _key_. Maybe use the title of the property subschema to determine the proposal string.
  • If the user selects the proposal, select the inserted name so the user can immediately type the intended, legal name.

These would be a significant improvement, and are probably worth doing on their own merit.

Name-Value Template Adapter

I'd like to add another possible solution: suggest templates defined in the property value context, and add some logic to adapt the template, so it automatically adds a template variable for the property name.

For example, indented one level under definitions in OpenAPI v2, we currently propose _key_.

image

What's expected here is a property with a user-specified name, and value conforming to a schema object. So we could say that schema object is the property value context, and we could promote templates from that context into the proposal list for the overall property:

image

To make this more interesting, we'll add a few schema template variants. And to make it more forward-looking, we'll add the first usability improvement suggested above: proposing (schema name) instead of _key_:

image

If the user inserts the "object schema" template, we would automatically adapt the template to provide a schemaName variable as the property name.

image

This should not require creating and maintaining a separate set of templates for the definitions context. We are just intelligently adapting templates from the property value context, so the expanded template includes the property name and the property value, indented underneath.

Context-Sensitivity

Template adaptation should be context-sensitive:

  • As described in "Basic Usability Improvements" above, the proposed name (i.e. the key in the key-value pair) should be appropriate to the context. Where a schema definition is expected, content assist should propose schema_name as a keyword. Where a schema property is expected, it should propose property_name. The Template Adapter should use that same proposed name as the variable placeholder. So in the expanded template for a property, the user would see something like:


    image

  • The template adapter should also have some way to specialize the proposed template name. A context-sensitive name prefix might be good enough. Or we could create some convention for variable replacement in the template name. For example:
    • When creating a schema definition under /components/schemas, we would want to propose the templates as "object schema", "primitive schema", etc.
    • When proposing those same templates in property context, we would want to present them as "property with object schema", "property with primitive schema", etc.

Array Item Template Adapter

We could use similar logic for array elements, like the parameters context. Right now we just propose - (array element prefix) in those contexts. Similarly, we could promote parameter templates into this code assist context, and propose those templates, auto-adapted to include the - prefix.

@tfesenko
Copy link
Member Author

@tedepstein I have a prototype (available on task/227) implementing the Basic Usability Improvements:
additionalpropertiesassist

It's almost ready, just need to clean up the code.
The Name-Value Template Adapter should also be relatively easy.

@tfesenko
Copy link
Member Author

@tedepstein I added code templates for named schemas. They are generated by Java code from code templates for schema bodies defined in templates.xml .
This is what they look like:
screen shot 2017-09-26 at 8 02 21 pm

@tedepstein
Copy link
Collaborator

@tfesenko , this looks good!

We have to somehow communicate that "schema" in the proposal list is not a keyword; it's a placeholder for a schema name that the user needs to type in. We provide a hint after insertion, by selecting the text so the user can overwrite, but I think it should also be more obvious in the proposal list. Please try using (schema name) as the proposal, and as the inserted/selected text.

@tedepstein
Copy link
Collaborator

@tfesenko, this should also work for properties of object schemas. Same problem in the current editor:

image

In this context, we want to propose (property name) in code assist and in the expanded templates. It should adapt the templates from the schema context, same as you've done for /components/schemas.

And we should take a look at the other "Patterned Fields" in the OAS2 and OAS3 specs, to see where else we can apply this same usability improvement. In OAS3 it looks like callbacks and security schemes are similarly named elements that could benefit from Name-Value Template Adapters. There may be others as well.

Finally, there are similar opportunities for array items.

@tfesenko , if you want, you can split this into separate issues for OAS2 vs. OAS3., separate issues for individual contexts, and/or separate issues for Name/Value contexts vs. Array Item contexts.

tfesenko added a commit that referenced this issue Sep 27, 2017
@tedepstein tedepstein changed the title Better code completion proposal for name/key of additionalProperties Better code completion proposal for name/key of patterned properties and array elements Sep 28, 2017
@tedepstein
Copy link
Collaborator

@tfesenko , I updated the spec above with additional context-sensitivity requirements. We will need to discuss the timing of this, as the full scope of work to be done is more than I originally thought through.

tfesenko added a commit that referenced this issue Oct 2, 2017
tfesenko added a commit that referenced this issue Oct 2, 2017
tfesenko added a commit that referenced this issue Oct 2, 2017
tfesenko added a commit that referenced this issue Oct 3, 2017
… format '(schema name)' and select it for editing
tfesenko added a commit that referenced this issue Oct 3, 2017
tfesenko added a commit that referenced this issue Oct 3, 2017
tfesenko added a commit that referenced this issue Oct 4, 2017
tfesenko added a commit that referenced this issue Oct 4, 2017
tfesenko added a commit that referenced this issue Oct 6, 2017
tfesenko added a commit that referenced this issue Oct 6, 2017
ghillairet added a commit that referenced this issue Oct 6, 2017
…perties

Add test to show that _key_ is still present in content assist for response content schema.
tfesenko added a commit that referenced this issue Oct 6, 2017
#227 Better code completion proposal for name/key of additional properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants