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

Search path for included files #618

Merged
merged 3 commits into from
Aug 20, 2022
Merged

Search path for included files #618

merged 3 commits into from
Aug 20, 2022

Conversation

eiswind
Copy link
Contributor

@eiswind eiswind commented Aug 14, 2022

I adjusted the search path for included files, in case that the current file is not at the root of the workspace.

I really would like to see that the completions for document attributes are either hidden or shown towards the end of the list, at least they (most of them) do not make much sense in the context of an include. I tried to figure that out, but I could not find the place where the AsciidocProvider gets registered. If I could get some directions here, I'll try to figure that out.

image

@ggrossetie
Copy link
Member

Hey, thanks for your contribution!
Could you please open an issue to describe what is the current behavior and what is the desired/expected behavior?

I really would like to see that the completions for document attributes are either hidden or shown towards the end of the list, at least they (most of them) do not make much sense in the context of an include. I tried to figure that out, but I could not find the place where the AsciidocProvider gets registered. If I could get some directions here, I'll try to figure that out.

I believe you can use/define the sortText property: https://stackoverflow.com/questions/56826661/how-to-force-order-in-vscode-extension-completionitems

I think we should do a couple of things:

  • group built-in document attributes: 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.
  • when requesting auto-completion on an include, image, video we should prioritize paths to local ressources then user-defined attributes (where the value can be used in a path), then built-in attributes usually used in path then other built-in attributes.
  • when requesting auto-completion on the image macro, we should prioritize files with image file extensions: svg, png, jpg, jpeg...
  • when requesting auto-completion on the video macro, we should prioritize files with video file extensions: mp4, mov...
  • when requesting auto-completion on the include macro, we should only show paths to text files (i.e., remove binary files)

What do you think? We should probably create an issue for each improvements to keep track of things.

@eiswind
Copy link
Contributor Author

eiswind commented Aug 14, 2022

Thanks for getting back so quickly.

I created two new issues to get started.

#620
#619

@eiswind
Copy link
Contributor Author

eiswind commented Aug 14, 2022

I'd love to see this little PR merged as well, as it allows to support non-root-folder asciidoc files.

@eiswind eiswind changed the title Issue #377: Search path for included files Search path for included files Aug 15, 2022
@ggrossetie
Copy link
Member

I'd love to see this little PR merged as well, as it allows to support non-root-folder asciidoc files.

Sure, I will test your fix locally in the next few days.

@ggrossetie
Copy link
Member

I can confirm that your fix/enhancement is working great!
Could you please fix the linter issue before I merge? You can run npm run lint locally. Thanks!

@eiswind
Copy link
Contributor Author

eiswind commented Aug 17, 2022

sure!

@ggrossetie ggrossetie merged commit e30084b into asciidoctor:master Aug 20, 2022
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.

2 participants