feat: handle documentation references both in source references and in alias catalog

[andsel]

• Closes #174

Feature description

Implements a new DOCS directive that permit insert a link or file reference that contains the documentation for the script. The documentation reference can be also assigned to an alias of a script and in that case takes precedence over the directive present in the script. The documentation reference can be opened with a newly added info subcommand, name docs, like:

jbang info docs <script|alias>

Describe changes at code level

The main change is to collect and resolve the reference present in the new DOCS directive in TagReader.getDocs. During JBang project creation or update the docs reference is collected and stored, to be later read in the ScriptInfo helper class. On the cli.Alias command a new --docs parameter is added to be accept a manually provided doc reference and store in the catalog. This catalog information is used in the ScriptInfo's initialization, after a resolution to a ResourceRef, to eventually overwrite the directive coming form the script.

How to test

Use a script like following for the testing part:

///usr/bin/env jbang "$0" "$@" ; exit $?
//DESCRIPTION This is a description test
//DOCS this_exists.txt
//DOCS ./this_exists.txt
//DOCS /tmp/this_exists.txt
//DOCS http://www.jbang.dev/documentation/guide/latest/index.html
//DOCS does-not-exist.txt

import static java.lang.System.*;

public class docsexample {

    public static void main(String... args) {
        out.println("Hello " + (args.length>0?args[0]:"World"));
    }
}

Build a new JBang distribution and and uncompress in tmp folder:

> ./gradlew distTar
> cp build/distributions/jbang-0.126.1.12.tar /tmp
> cd /tmp
> tar xf jbang-0.126.1.12.tar

In the same tmp folder create a new file this_exists.txt with some content.

Browse doc from script

/tmp/jbang-0.126.1.12/bin/jbang info docs docsexample.java

Opens a browser or an editor.

Browse doc from alias

Add an alias overriding the file's contained directive and check it opensù

/tmp/jbang-0.126.1.12/bin/jbang alias add --docs https://www.jbang.dev/documentation/guide/latest/javaversions.html docsexample.java
/tmp/jbang-0.126.1.12/bin/jbang alias list --format=json
/tmp/jbang-0.126.1.12/bin/jbang info docs docsexample

Check it opens the web browser to https://www.jbang.dev/documentation/guide/latest/javaversions.html .

Remove the alias

/tmp/jbang-0.126.1.12/bin/jbang alias remove docsexample
/tmp/jbang-0.126.1.12/bin/jbang alias list --format=json

Add an alias without a docs ref forcing

/tmp/jbang-0.126.1.12/bin/jbang alias add docsexample.java
/tmp/jbang-0.126.1.12/bin/jbang info docs docsexample

Verify if opens the editor or the browser pointing to the resource defined in the first DOCS directive

Dev tasks

• [x] complete the implementation of display the doc reference
• [x] cover with unit test
• [x] implement the catalog retrieval, giving it precedence over source file directive
• [ ] check if integration tests are feasible
• [ ] update the documentation

Summary by Sourcery

Implement support for documentation references via a new DOCS directive and alias option, and expose them through an info docs command to list and optionally open linked documentation.

New Features:

• Introduce a DOCS directive to annotate scripts with documentation file or URL references
• Add an info docs subcommand to display and open documentation references for scripts and aliases
• Enable specifying documentation references for aliases using a new --docs option

Enhancements:

• Extend the project and alias catalog to collect and resolve DocRef instances
• Prioritize alias-provided documentation references over inline script directives

Tests:

• Add unit tests for file and URL documentation resolution
• Add integration tests to verify info docs output and browser-opening behavior

[quintesse]

@andsel I updated your PR to make it work. The issue was the URL you were testing with, for now it must refer to an actual file or page

[andsel]

@quintesse thank's a lot for the addition, so it has to always download the url and then let it open in the browser.

I'll proceed by aligning the documentation.

[quintesse]

so it has to always download the url and then let it open in the browser.

No, that's just a temporary implementation detail that can be fixed later. It's of no consequence to the docs feature.

[quintesse]

@andsel if you rebase again against the latest version of "main" then you'll have access to lazy resource refs that I just added: https://github.com/jbangdev/jbang/pull/2031

You can update the code like:

prj.setDocs(tagReader.getDocs(LazyResourceResolver.lazy(resolver1)).orElse(null));

Although perhaps we can even use it for the shared resource resolver (but that would have to be thoroughly tested):

ResourceResolver resolver1 = LazyResourceResolver.lazy(new SiblingResourceResolver(resourceRef, ResourceResolver.forResources()));

[andsel]

@quintesse which is the best place to document this feature?

• should I add a new section (like caching.adoc) to describe what's this documentation feature it?
• -or- add paragraph in the "Aliases & Catalog" ? I propose this because the feature also touches the aliases creation, when a an alias can be created with a specific link to the documentation for it, which overrides the tag present in the script.

[quintesse]

which is the best place to document this feature?

To be honest, there currently is no "best place". We are sorely missing some organization in our docs.

should I add a new section (like caching.adoc) to describe what's this documentation feature it?

It's too small a feature IMO to merit its own page

-or- add paragraph in the "Aliases & Catalog" ? I propose this because the feature also touches the aliases creation, when a an alias can be created with a specific link to the documentation for it, which overrides the tag present in the script.

Well, then we should add all options to that page because all tags can be overridden by passing arguments to the alias. So no I don't think that's a good option either.

The real issue here is, IMO, that we're missing a page that simply shows what //-tags are available and what they are used for. Yes, a bunch of them are sprinkled throughout the docs but we're missing something that says: these are all the ones and this is what they do.

In such a page we could then show for each tag what the corresponding command line option is (eg //JAVA vs --java). They'd also have a short description saying what each one is used for and for the ones that have additional information in other doc pages we could add links to those pages (eg, the section for //JAVA (--java) could link to https://www.jbang.dev/documentation/guide/latest/javaversions.html)

[andsel]

@quintesse

but that would have to be thoroughly tested I'll rebase and test

About the documentation point:

we're missing a page that simply shows what //-tags

I've created the issue #2032 to address this (I may work on it, so eventually please assign to me)

[quintesse]

Btw, I'd be willing to merge this PR without the docs and leave that to be done in #2032

[andsel]

@quintesse thanks a lot for your all your guidance on this! :-D

[quintesse]

@quintesse thanks a lot for your all your guidance on this! :-D

And thank you for your implementation and in moving this issue forward! :-)

[maxandersen]

trying this out and having a few issues:

• failing on file with no jbang/docs in them; it should help/guide the user not just fail.

just jbang info docs examples/readme.adoc
[jbang] [ERROR] Cannot invoke "dev.jbang.source.ResourceRef.isURL()" because "info.docs" is null
[jbang] Run with --verbose for more details. The --verbose must be placed before the jbang command. I.e. jbang --verbose run [...]

- fail on file with multiple //DOCS and first one is missing

This just fails on the file not being there - but there are multiple others that does exist...can't / shouldn't we show it?

As mentioned on #174 "I would treat it as first (or last dependent on pov) one wins and everything else is secondary." - in case of first one not being there I would print/show the other options.

- auto-open on urls

I'm not sure its super safe to open browser or default editor on a file/resource - could be tricky...I would suggest we give a chance to cancel it. Either asking expliclitly or do the "wait 5 seconds" to open as we do for trust checking.

Could still have a `jbang info docs --open test.java` to just open directly 

- ignoring multiple //DOCS
I would print all the docs found, not just open first one.

- not a blocker, but should the list of docs found not be listed in `jbang info tools` as a new key info?

[quintesse]

This just fails on the file not being there - but there are multiple others that does exist...can't / shouldn't we show it?

Dunno, that what likely make the code a whole lot more complex for something that shouldn't happen (a file shouldn't have multiple //DOCS). It would be a bit like a file having both a //JAVA 170+ and a //JAVA 17+ and then saying "why can't it just pick the one that is correct?". Yes, we could, but should we?

in case of first one not being there I would print/show the other options.

~But that would require doing a resolution at a point that you don't even know if the user is going to want the information. (You don't know if they'll be running jbang info or jbang run). Again, related to the example above, if we'd try the different ??JAVA options we'd be doing possibly several failing remote request only to find out that we want Jdk 17 which was installed already.~

I would print all the docs found, not just open first one.

Ok, this would mean that we simply allow multiple //DOCS tags, just like //SOURCES and //FILES and that "docs" would be an array of values. That could work. And if we make them all Lazy we can delay resolution to the last possible moment.

Still, if you do that I would find it pretty weird that we'd only allow you to open a single one using --open.

Also, if the docs reference is for example ./docs.html the output of info docs (without --open) would be pretty useless. (And it would be replicated verbatim by the info tools command, once it's updated, as you mentioned)

I don't immediately have a good solution, I just want to point out that what we have now works well IMO and is easy to understand. These new suggestions seems to make things less well defined and less useful.

[andsel]

These are my thoughts on the multi-docs theme: I think that printing all the docs reference found could be useful, in particular I could imagine a case where there is a //DOCS in the source file and another from the alias catalog. In such case it's selected the one coming from the catalog and would help the user to understand why wasn't selected the one provided in the source file.

• fail on file with multiple //DOCS and first one is missing While the definition of missing is obvious on a local file (the file doesn't exist) when it's a URL we have 2 choices:

1. assure that the URL return an HTTP 2xx code (following eventual 302 and 301 codes)
2. consider the URL as always a valid reference.

In case 1 we have to navigate the URL, and could be not a stable result, in case the server or network are unreachable. In case 2. (as it is today) we let the user to face the eventual error.

but should the list of docs found not be listed in jbang info tools as a new key info?

Also if we decide to don't follow the multi-docs path, the information for the single doc is useful in the output of that command.

• auto-open on urls

I'm not sure its super safe to open browser or default editor on a file/resource - could be tricky...I would suggest we give a chance to cancel it. Either asking expliclitly or do the "wait 5 seconds" to open as we do for trust checking.

failing on file with no jbang/docs in them; it should help/guide the user not just fail.

This is a bug fixed with commit 94939af6361e797be6555173f6375552887b521f

Could still have a jbang info docs --open test.java to just open directly

I agree with the fact to request the user the acknowledgment to open an url.

[andsel]

@quintesse I think the commit 7984ce528c6cefa1d970753dbabb332ce08a0309 broke the opening the doc reference defined in an alias.

So for example if I have a script which declare a DOCS directive like:

//DOCS http://www.jbang.dev/documentation/guide/latest/index.html

and then with the following command I define an override of the doc reference in an alias pointing to the same script, like in

jbang alias add --docs https://www.jbang.dev/documentation/guide/latest/javaversions.html itests/docsrun.java

When I open the docs reference for the alias with:

jabang info docs docsrun

I would expect it open the https://www.jbang.dev/documentation/guide/latest/javaversions.html and not the one from the script directive http://www.jbang.dev/documentation/guide/latest/index.html.

As I can understand the problem comes from this line: https://github.com/jbangdev/jbang/pull/2013/commits/7984ce528c6cefa1d970753dbabb332ce08a0309#diff-0ed5e949a3d2d5b76e97b0a03e821cf4cb9d71645790f03c4eb28699f5f8e62cR674

docs is always null, while it should check alias.docs. Please could you confirm that?

[quintesse]

docs is always null, while it should check alias.docs.

Indeed, the if in line 674 is inverted, it should check for == null, just like the rest (and as you state right now that will always be true, still I'd like the if to remain in case the docs ever is not null in the future).

[andsel]

@quintesse

still I'd like the if to remain in case the docs ever is not null in the future

Do you mean that when updateFromAlias it's always expected that doc field is null? The only case I could imagine where it's non null, is when updateFromAlias is invoked twice.

In this case, should it raise a sort of programmatic error? (which is an error that the user can't do anything and we can't add any valuable call to action for him).

[quintesse]

Do you mean

No, I mean that I want that if-statement to remain there, even if, for now, it's result will always be true. Just humor me :-)

[andsel]

No, I mean that I want that if-statement to remain there, even if, for now, it's result will always be true. Just humor me :-)

@quintesse done with 95b7a9a14a3919ae76ca15a92bea36d737d14788

[quintesse]

Now we have to finish the discussion about one doc, multiple docs, auto-open or not etc.

To me multiple docs don't make sense when you also have a way to open them, but you're only going to open one.

To me printing out a bunch of URLs or file paths also don't make sense. What is the use of that? To do anything useful with that you'd have to copy & paste them into a browser or reader.

If you run info docs you know you're going to open a browser window, it's not a command that you can run by accident. (Except perhaps the very first time when you're new to JBang)

So my vote is for keeping things like they are now, it makes the most sense to me.

Edit: NB, I want to clarify: it's not that I don't see why people might like wanting to add multiple doc links, it's just that I don't see a nice way to make that feature really useful and make sense

[maxandersen]

My terminal when printing filenames/urls lets me click on them directly and/or pass it to other tools to process.

That + I can't really know upfront which docs are relevant for me for a set of combined scripts makes it for me makes it it relevant to at least be able to show them.

One could argue that the automatic open is the challenge here?

if it was not automatically opened and jbang info docs did this:

jbang info doc.java
this_do_not_exists.txt
https://xam.dk/notthere
this_exists.txt
/tmp/this_exists.txt
http://www.jbang.dev/documentation/guide/latest/index.html
does-not-exist.txt

I as a user could pick the one relevant to me, click on it and it opens.

and if not auto open you do:

jbang info doc.java --open
Opening first out of 7 doc: this_do_not_exists.txt
To see all run jbang info doc.java

or if we flip it back to current:

jbang info doc.java
Opening first out of 7 doc: this_do_not_exists.txt
To see all run jbang info doc.java --no-open

(--no-open is also used for jbang edit)

and

jbang info doc.java --no-open
this_do_not_exists.txt
https://xam.dk/notthere
this_exists.txt
/tmp/this_exists.txt
http://www.jbang.dev/documentation/guide/latest/index.html
does-not-exist.txt

...also, remember how we deal with maven repos - they are key/value pairs but key is optional.

we could allow:

//DOCS javadoc=https://javadoc.io/whereever //DOCS website=https://.... //DOCS main=https://...

and then it would be possible to do jbang info doc.java javadoc ...and on jbang info doc.java we would show main if avaiable otherwise first in list.

for me there are plenty of reasosn why multiple docs are relevant and why having info also in jbang info tools makes sense.

[andsel]

Regarding the multi vs single doc question. I thought that original intention of the feature was to drive the user to sort of landing page for the script/project, where it could find the description of the tool plus other pointers. In this spirit having multiple DOCS is an error, that could be possible, and have to work with that.

@maxandersen in case you want to provide multiple doc links to the user, then the script developer can provide a single page with multiple link, a sort of TOC. We can inform the user that we are opening/displaying the first doc reference of a list, in case there a many, but just to let him know why we selected that URL/file in case he was editing a doc directive and not seeing what he expected.

[quintesse]

Re:

jbang info doc.java
this_do_not_exists.txt
https://xam.dk/notthere
this_exists.txt
/tmp/this_exists.txt
http://www.jbang.dev/documentation/guide/latest/index.html
does-not-exist.txt

@maxandersen I can understand that we call all technically do this, but my question, with capital letters en question marks on purpose is, WHY???

Who on earth is ever going to use any of that? We're solving an issue from 5 years ago of something that would "nice to have" and we're turning it into a full-fledged documentation feature challenging the library of congress... for what? For scripts that are 10 lines long?

Edit: to me this is about having the feature where you can run info docs <script> and docs pop up. People can add a dozen extra links as comment in the code if they want, as we can do with any code file. But the //DOCS line is specifically for JBang and specifically to do one simple thing.

[maxandersen]

It matches all the other interactions we have. I got it implemented with very few chances and fixing a few bugs in meantime. I'll push it later tonight or tomorrow morning so you can see.

[maxandersen]

@andsel good work - I pushed commits to do the named docs via key/value as discussed above.

Now it does this:

This is a description test
main:
  file:///Users/manderse/code/jbangdev/jbang/docsexample.java
  https://xam.dk/notthere
  http://www.jbang.dev/documentation/guide/latest/index.html
  does-not-exist.txt (not found)
javadoc:
  /tmp/this_exists.txt (not found)

it uses ansi colors to make it look pretty :)

atm it defaults to not open...but I can be convinced to flip it so one would need to use --no-open but let me know what you all think.

[quintesse]

Great! Just a quick question: is it important to show the "not found" message? It could possibly save a lot of requests that are not really necessary anyway, right? (We don't need the eocs to be locally available in the cache)

[quintesse]

Also, don't forget to add the docs to the info tools output, like we discussed

[maxandersen]

Great! Just a quick question: is it important to show the "not found" message? It could possibly save a lot of requests that are not really necessary anyway, right? (We don't need the eocs to be locally available in the cache)

I need to resolve it to give the path to open. So I know the state anyway.

[quintesse]

Great! Just a quick question: is it important to show the "not found" message? It could possibly save a lot of requests that are not really necessary anyway, right? (We don't need the eocs to be locally available in the cache)

I need to resolve it to give the path to open. So I know the state anyway.

No, for URLs you don't need the path. So for URLs you're now actually downloading the HTML page. That's why @andsel first checked if the ref was a URL and only if it was not he would get the Path. (The whole point in creating Lazy resolvers was so we didn't have to resolve and download a bunch of unneeded files :-) )

[maxandersen]

And thats what the code does atm , right?

[andsel]

@andsel good work -

@maxandersen thanks, I would have been more effective to the project, but at the end opened the discussion on this. Hope to be more helpful in another PR.