asciidoctor-vscode icon indicating copy to clipboard operation
asciidoctor-vscode copied to clipboard

group built-in document attributes

Open eiswind opened this issue 3 years ago • 11 comments
trafficstars

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.

eiswind avatar Aug 14 '22 14:08 eiswind

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.

ggrossetie avatar Aug 14 '22 20:08 ggrossetie

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)?

eiswind avatar Aug 15 '22 07:08 eiswind

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.

ggrossetie avatar Aug 15 '22 09:08 ggrossetie

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

eiswind avatar Aug 15 '22 13:08 eiswind

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.

eiswind avatar Aug 18 '22 08:08 eiswind

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.

ggrossetie avatar Aug 18 '22 18:08 ggrossetie

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.

eiswind avatar Aug 19 '22 07:08 eiswind

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.

ggrossetie avatar Aug 20 '22 12:08 ggrossetie

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

Scroll down a bit

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

eiswind avatar Aug 21 '22 08:08 eiswind

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 avatar Aug 21 '22 15:08 ggrossetie

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`...

ggrossetie avatar Aug 21 '22 15:08 ggrossetie