asciidoctor / asciidoctor-vscode

AsciiDoc support for Visual Studio Code using Asciidoctor
Other
328 stars 97 forks source link

group built-in document attributes #619

Open eiswind opened 2 years ago

eiswind commented 2 years ago

I would like to suggest to show only relevant suggestions on include/image/video. Preferably the file suggestions should appear before the attributes that may be relevant.

@Mogztter suggested

https://docs.asciidoctor.org/asciidoc/latest/attributes/document-attributes-ref/

then we can decide which group(s) we want to enable on which context for instance: compliance, localization and numbering attributes don't make much in the context of an include, image, video, link macro.

As far as I understand the workings, those items are provided by AttributeReferenceProvider

I do not have an idea how to check the context there, as the includes check that on their own in AsciidocProvider

I have seen prefixes beeing used to achieve sortOrder, but I don't have an idea on filtering by context. But maybe that is not even necessary when we have a meaningful order.

ggrossetie commented 2 years ago

Built-in attributes are provided by: https://github.com/asciidoctor/asciidoctor-vscode/blob/master/src/features/builtinDocumentAttributeProvider.ts with relies on https://github.com/asciidoctor/asciidoctor-vscode/blob/master/src/features/builtinDocumentAttribute.json

AttributeReferenceProvider provides auto-completion when you want to reference an attribute (i.e. {my-attribute}).

I do not have an idea how to check the context there, as the includes check that on their own in AsciidocProvider

VS code gives you the document (vscode.TextDocument) and the position (vscode.Position) when auto-completion is requested (i.e., when provideCompletionItems is called). So you can get the line and check if the line starts by include:: or image:: or video:: and use that information to change the sortOrder and/or return only meaningful completions items. Does it make sense?

I have seen prefixes beeing used to achieve sortOrder, but I don't have an idea on filtering by context. But maybe that is not even necessary when we have a meaningful order.

I think we should use numeric values: 10, 20, 30.... if two items have the same sortOrder then I think it will use the label. So if we want to show files before attributes then files will be assigned 10 as sortOrder and attributes will be assigned 20 as sortOrder.

eiswind commented 2 years ago

When the built-in attributes come from builtinDocumentAttributeProvider and the file suggestions come from AsciidocProvider the context check is (current state) made in AsciidocProvider. Should we set the sort prefix on the attributes always (does that do any harm?) or do we need to have the context check in all the places (what I think would make things even harder to reason about)?

ggrossetie commented 2 years ago

I think that's fine, we can set an arbitrary value on built-in attributes and use a lower (or greater) value on other completion items.

eiswind commented 2 years ago

I'll try to set up a PR in the next few days.

eiswind commented 2 years ago

When trying to set this up, I found that in the context on an include the attributes are provided by AttributeReferenceProvider which uses document.getAttributes() . That completely makes sense, but it would be of no help to group the attributes from BuiltinDocumentAttributeProvider as they are not shown here anyway.

If it is of interest to have the grouping for other reasons I could open up thePR, as I made the changes to buildinDocumentAttribute.jsonalready, but I don't see how to make any sense of it in the current setup.

ggrossetie commented 2 years ago

If it is of interest to have the grouping for other reasons I could open up the PR

I haven't tested it recently but I think you can show all the auto-completion items (from all the providers) if you explicitly type Ctrl+Space.

eiswind commented 2 years ago

https://github.com/asciidoctor/asciidoctor-vscode/blob/b65f1c171ce828e71f784cfdf8c5f2bc6e429f66/src/features/builtinDocumentAttributeProvider.ts#L12-L19

The attributes from the json file are only shown, when the line starts with a colon.

Actually there are also duplicates then coming from the AttributeReferenceProvider.

ggrossetie commented 2 years ago

The attributes from the json file are only shown, when the line starts with a colon.

Oh that's right!

Actually there are also duplicates then coming from the AttributeReferenceProvider.

Do you have an example? We might need to use a Set to remove duplicates. Also, I think we need to check the context when providing document attributes as completion items.

eiswind commented 2 years ago

Bildschirmfoto vom 2022-08-21 10-41-53

Scroll down a bit

Bildschirmfoto vom 2022-08-21 10-42-09

ggrossetie commented 2 years ago

In this case, built-in attributes must come first. Having said that, it does not make sense to display attribute references unless you want to use the value of an attribute in an attribute value:

:url-asciidoctor-docs: https://docs.asciidoctor.org
:url-asciidoctorjs-doc: {url-asciidoctor-docs}/asciidoctor.js/latest/

Anyway, we should hide attribute references if the line only contains :. Similarly, it does not make sense to display built-in attribute when defining an attribute value, so if the line is not : then we should not display them (but I think that's already the case)

ggrossetie commented 2 years ago

To clarify, the name is identical but the purpose is not the same so they are not duplicates.

// Define a built-in attribute
:app-name: foo

// Reference the app-name attribute 
Install {app-name} using `npm`...