Generate reference links for painless API #22775

nik9000 · 2017-01-24T21:56:58Z

Adds "Appendix B. Painless API Reference", a reference of all classes
and methods available from Painless. Removes links to java packages
because they contain methods that we don't expose and don't contain
methods that we do expose (the ones in Augmentation). Instead this
generates a list of every class and every exposed method using the same
type information available to the
interpreter/compiler/whatever-we-call-it. From there you can jump to
the relevant docs.

Right now you build all the asciidoc files by running

gradle generatePainlessApi

These files are expected to be committed because we build the docs
without running gradle.

You can also run it in an IDE safely if you pass the path to the
directory in which to generate the docs as the first parameter. It'll
blow away the entire directory an recreate it from scratch so be careful.

And then you can build the docs by running something like:

../docs/build_docs.pl --out ../built_docs/ --doc docs/reference/index.asciidoc --open

That is, if you have checked out https://github.com/elastic/docs in
../docs. Wait a minute or two and your browser will pop open in with
all of Elasticsearch's reference documentation. If you go to
http://localhost:8000/painless-api-reference.html you can see this
list. Or you can get there by following the links to Modules and
Scripting and Painless and then clicking the link in the paragraphs
below titled Appendix B. Painless API Reference.

I like having these in asciidoc because we can deep link to them from the
rest of the guide with constructs like
<<painless-api-reference-Object-hashCode-0>> and
<<painless-api-reference->> and we get link checking. Then the only
brittle link maintenance bit is the link generation for javadoc. Which
sucks. But I think it is important that we link to the methods directly
so they are easy to find.

Adds "Appending B. Painless API Reference", a reference of all classes and methods available from Painless. Removes links to java packages because they contain methods that we don't expose and don't contain methods that we do expose (the ones in Augmentation). Instead this generates a list of every class and every exposed method using the same type information available to the interpreter/compiler/whatever-we-call-it. From there you can jump to the relevant docs. Right now you build all the asciidoc files by running ``` gradle generatePainlessApi ``` These files are expected to be committed because we build the docs without running `gradle`. Also changes the output of `Debug.explain` so that it is easy to search for the class in the generated reference documentation. You can also run it in an IDE safely if you pass the path to the directory in which to generate the docs as the first parameter. It'll blow away the entire directory an recreate it from scratch so be careful. And then you can build the docs by running something like: ``` ../docs/build_docs.pl --out ../built_docs/ --doc docs/reference/index.asciidoc --open ``` That is, if you have checked out https://github.com/elastic/docs in `../docs`. Wait a minute or two and your browser will pop open in with all of Elasticsearch's reference documentation. If you go to `http://localhost:8000/painless-api-reference.html` you can see this list. Or you can get there by following the links to `Modules` and `Scripting` and `Painless` and then clicking the link in the paragraphs below titled `Appendix B. Painless API Reference`. I like having these in asciidoc because we can deep link to them from the rest of the guide with constructs like `<<painless-api-reference-Object-hashCode-0>>` and `<<painless-api-reference->>` and we get link checking. Then the only brittle link maintenance bit is the link generation for javadoc. Which sucks. But I think it is important that we link to the methods directly so they are easy to find.

nik9000 · 2017-01-24T21:58:36Z

I haven't added the docs tag because this also contains an enhancement to painless to fix Debug.explain to work better with the docs this generates. @clintongormley, please fix the tags to something you think'd look right in the release notes.

I also am not including all of the generated docs in this PR for now. I'll regenerate after a review. For now I've included some of the generated asciidoc so we can look at it and talk about it.

nik9000 · 2017-01-24T21:59:21Z

docs/reference/modules/scripting/painless-debugging.asciidoc

@@ -38,23 +38,24 @@ POST /hockey/player/1/_explain
 // we have to override it to get a normal shaped response

 Which shows that the class of `doc.first` is
-`org.elasticsearch.index.fielddata.ScriptDocValues$Longs` by responding with:
+`org.elasticsearch.index.fielddata.ScriptDocValues.Longs` by responding with:


Changed this to match the painless_class which is what we document in the reference.

nik9000 · 2017-01-24T21:59:51Z

docs/reference/modules/scripting/painless-debugging.asciidoc

@@ -90,19 +92,5 @@ The response looks like:
 // TESTRESPONSE[s/\.\.\./"script_stack": $body.error.caused_by.script_stack, "script": $body.error.caused_by.script, "lang": $body.error.caused_by.lang, "caused_by": $body.error.caused_by.caused_by, "reason": $body.error.caused_by.reason/]
 // TESTRESPONSE[s/"to_string": ".+"/"to_string": $body.error.caused_by.to_string/]

-// TODO we should build some javadoc like mashup so people don't have to jump through these hoops.


This PR is this TODO.

nik9000 · 2017-01-24T22:00:49Z

docs/reference/painless-api-reference.asciidoc

+worth going to the version that matches the version of Java you are using to
+run Elasticsearch just in case.
+
+include::painless-api-reference/index.asciidoc[]


This file is maintained by hand. Everything in the painless-api-reference directory, including index.asciidoc is generated.

nik9000 · 2017-01-24T22:01:28Z

docs/reference/painless-api-reference/Math.asciidoc

+Rebuild by running `gradle generatePainlessApi`.
+////
+
+* [[painless-api-reference-Math]]Math[afterthought]##(link:{java8-javadoc}/java/lang/Math.html#Math[reference])##


I decided to generate a file per class rather than dump them all in the same file because I thought these files were dense enough as is.

nik9000 · 2017-01-24T22:01:59Z

modules/lang-painless/src/main/java/org/elasticsearch/painless/Debug.java

-/**
- * Utility methods for debugging painless scripts that are accessible to painless scripts.
- */
-public class Debug {


Moved so it is included in the public javadocs.

nik9000 · 2017-01-24T22:02:16Z

modules/lang-painless/src/main/java/org/elasticsearch/painless/Definition.java

            this.methods = Collections.unmodifiableMap(methods);
            this.getters = Collections.unmodifiableMap(getters);
            this.setters = Collections.unmodifiableMap(setters);
        }
+
+        public Struct getStruct() {


Added so we can get the painless_class from Debug.explain.

nik9000 · 2017-01-24T22:02:33Z

modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessExplainError.java

+ * Thrown by {@link Debug#explain(Object)} to explain an object. Subclass of {@linkplain Error} so it cannot be caught by painless
+ * scripts.
+ */
+public class PainlessExplainError extends Error {


Extracted so it isn't in the public API.

nik9000 · 2017-01-24T22:03:06Z

modules/lang-painless/src/main/java/org/elasticsearch/painless/ScriptImpl.java

-        // Note that it is safe to catch any of the following errors since Painless is stateless.
-        } catch (Debug.PainlessExplainError e) {
-            throw convertToScriptException(e, e.getMetadata());
+        } catch (PainlessExplainError e) {


I just fix the comment location here.

nik9000 · 2017-01-24T22:03:19Z

modules/lang-painless/src/main/java/org/elasticsearch/painless/api/Debug.java

+    /**
+     * Throw an {@link Error} that "explains" an object.
+     */
+    public static void explain(Object objectToExplain) throws PainlessExplainError {


Moved so it is part of the public API.

nik9000 · 2017-01-24T22:04:40Z

I'm thinking at some point of writing a test that validates that the generated documentation is up to date but I wanted to get this up for review now. I'm happy to add the test as part of this PR if someone thinks I should or do it as a followup.

clintongormley

In general looks good. I'm wondering if we can improve the asciidoc layout a bit. What do you think of something like this:

for which the asciidoc looks like this:

[[painless-api-reference-AttributedString]]++link:{java8-javadoc}/java/text/AttributedString.html#AttributedString[AttributedString]++::
* [[painless-api-reference-AttributedString-AttributedString-1]]++link:{java8-javadoc}/java/text/AttributedString.html#AttributedString%2Djava.lang.String%2D[AttributedString](<<painless-api-reference-String,`String`>>)++ (link:{java9-javadoc}/java/text/AttributedString.html#AttributedString%2Djava.lang.String%2D[java 9])
* [[painless-api-reference-AttributedString-AttributedString-2]]++link:{java8-javadoc}/java/text/AttributedString.html#AttributedString%2Djava.lang.String%2Djava.util.Map%2D[AttributedString](<<painless-api-reference-String,String>>, <<painless-api-reference-Map,Map>>)++ (link:{java9-javadoc}/java/text/AttributedString.html#AttributedString%2Djava.lang.String%2Djava.util.Map%2D[java 9])

clintongormley · 2017-01-25T08:43:20Z

docs/reference/index.asciidoc

+:elasticsearch-javadoc: https://artifacts.elastic.co/javadoc/org/elasticsearch/elasticsearch/{version}
+:painless-javadoc: https://artifacts.elastic.co/javadoc/org/elasticsearch/painless/lang-painless/{version}
+endif::[]
+


clintongormley · 2017-01-25T08:43:50Z

docs/reference/index.asciidoc

@@ -63,3 +84,5 @@ include::glossary.asciidoc[]
 //////

 include::redirects.asciidoc[]
+
+include::painless-api-reference.asciidoc[]


I'd move the include above redirects

Makes sense.

clintongormley · 2017-01-25T08:45:53Z

docs/reference/painless-api-reference.asciidoc

@@ -0,0 +1,17 @@
+["appendix",role="exclude",id="painless-api-reference"]


role="exclude" means "don't add it to the ToC, but we do want it in the ToC, so that should be removed

Ah. When I first wrote this I was going under the assumption that we didn't want it in the ToC but I'm not really sure why I assumed that...

clintongormley · 2017-01-25T09:11:18Z

docs/reference/painless-api-reference/List.asciidoc

+////
+
+* [[painless-api-reference-List]]List[afterthought]##(link:{java8-javadoc}/java/util/List.html#List[reference])##
+** [[painless-api-reference-List-add-2]]void link:{java8-javadoc}/java/util/List.html#add%2Dint%2Djava.lang.Object%2D[add](int, def) (link:{java9-javadoc}/java/util/List.html#add%2Dint%2Djava.lang.Object%2D[java 9])


what is afterthought?

nik9000 · 2017-01-25T13:12:16Z

. What do you think of something like this

I don't want to make clicking on the name of the class jump to the class's javadoc because I think the class level javadoc (as opposed to the method level javadoc) can be a bit confusing. I added that (reference) link with the class level javadoc with the afterthought class because I wanted it there but wanted it smaller. afterthought is just:

diff --git a/resources/web/styles.css b/resources/web/styles.css
index 7dd5235..658a2b9 100644
--- a/resources/web/styles.css
+++ b/resources/web/styles.css
@@ -639,3 +639,7 @@ p.remark {
 #guide .abstract .page_header {
     width: 100%;
 }
+
+#guide .afterthought {
+    font-size: .6em;
+}

If we already have a class for this or would prefer to do it in another way I'm fine with whatever works.

It looks like the other asciidoc layout change you propose is the ++s. Those make sense to me.

clintongormley · 2017-01-25T14:22:50Z

with the afterthought class

I didn't know that syntax worked! I thought you'd have to specify a role=

I don't want to make clicking on the name of the class jump to the class's javadoc because I think the class level javadoc (as opposed to the method level javadoc) can be a bit confusing.

I found the (reference) after each class visually noisy (and not accessible to eg screen readers because all of the links have the same text). i'd prefer the class names to be the links - if people find themselves confused they can always just go back. Or an alternative (given that the methods and constructors are already linked) is just not to have a link there.

It looks like the other asciidoc layout change you propose is the ++s. Those make sense to me.

I also changed the class name to use a definition list:

`Byte:: `

and the methods and constructors to use a single * instead of **

nik9000 · 2017-01-25T18:41:54Z

Or an alternative (given that the methods and constructors are already linked) is just not to have a link there.

I think I'll do that.

I also changed the class name to use a definition list:

Cool. That makes sense to me.

It is confusing and break screen readers.

nik9000 · 2017-01-25T19:19:45Z

@jdconrad can you have a look at the changes that I made to the java code here? I don't think the doc generating class is all that interesting, but the rest of the java changes could use a look.

jdconrad

Sorry for the delay. LGTM! This is going to be a large improvement for our users, so a huge thanks @nik9000.

nik9000 · 2017-01-25T19:31:44Z

Sorry for the delay. LGTM! This is going to be a large improvement for our users, so a huge thanks @nik9000.

I'm genuinely excited to get this in! I'll wait another day in case @clintongormley has more ideas. I believe I've made all the changes he asked for at this point.

Thanks for looking at the Java code. I wanted to make sure I wasn't doing anything crazy but I think it is fairly safe because none of this is exposed to the scripts.

clintongormley · 2017-01-26T09:03:50Z

LGTM Nik! Thanks for doing this.

I have one remaining question, unsure about it in my own mind (and shouldn't impact this PR). It's a long page and I'm wondering about making it easier to navigate, eg listing the class names at the top with links to the relevant section. Like i say, I'm undecided because users have scroll bars and find-in-this-page anyway, so maybe the extra clutter isn't worth it.

nik9000 · 2017-01-26T14:33:32Z

Like i say, I'm undecided because users have scroll bars and find-in-this-page anyway, so maybe the extra clutter isn't worth it.

Yeah, I thought the same thing. My hope is that folks find their way to this page either by deep linking or by find-in-this-page style operations.

nik9000 · 2017-01-26T15:45:43Z

Thanks for reviewing @clintongormley and @jdconrad!

master: 8a2d424 a383bc1
5.x: 6fe7a17 74dcd73

Adds "Appending B. Painless API Reference", a reference of all classes and methods available from Painless. Removes links to java packages because they contain methods that we don't expose and don't contain methods that we do expose (the ones in Augmentation). Instead this generates a list of every class and every exposed method using the same type information available to the interpreter/compiler/whatever-we-call-it. From there you can jump to the relevant docs. Right now you build all the asciidoc files by running ``` gradle generatePainlessApi ``` These files are expected to be committed because we build the docs without running `gradle`. Also changes the output of `Debug.explain` so that it is easy to search for the class in the generated reference documentation. You can also run it in an IDE safely if you pass the path to the directory in which to generate the docs as the first parameter. It'll blow away the entire directory an recreate it from scratch so be careful. And then you can build the docs by running something like: ``` ../docs/build_docs.pl --out ../built_docs/ --doc docs/reference/index.asciidoc --open ``` That is, if you have checked out https://github.com/elastic/docs in `../docs`. Wait a minute or two and your browser will pop open in with all of Elasticsearch's reference documentation. If you go to `http://localhost:8000/painless-api-reference.html` you can see this list. Or you can get there by following the links to `Modules` and `Scripting` and `Painless` and then clicking the link in the paragraphs below titled `Appendix B. Painless API Reference`. I like having these in asciidoc because we can deep link to them from the rest of the guide with constructs like `<<painless-api-reference-Object-hashCode-0>>` and `<<painless-api-reference->>` and we get link checking. Then the only brittle link maintenance bit is the link generation for javadoc. Which sucks. But I think it is important that we link to the methods directly so they are easy to find. Relates to #22720

* master: (47 commits) Remove non needed import use expectThrows instead of manually testing exception Fix checkstyle and a test Update after review Read ec2 discovery address from aws instance tags Invalidate cached query results if query timed out (elastic#22807) Add remaining generated painless API Generate reference links for painless API (elastic#22775) [TEST] Fix ElasticsearchExceptionTests Add parsing method for ElasticsearchException.generateThrowableXContent() (elastic#22783) Improve connection closing in `RemoteClusterConnection` (elastic#22804) Docs: Cluster allocation explain should be on one page Remove DFS_QUERY_AND_FETCH as a search type (elastic#22787) Add repository-url module and move URLRepository (elastic#22752) fix date-processor to a new default year for every new pipeline execution. (elastic#22601) Add tests for top_hits aggregation (elastic#22754) [TEST] Added this for 93a28b0 submitted via elastic#22772 Fix typo in comment in OsProbe.java Add new ruby search library to community clients doc (elastic#22765) RangeQuery WITHIN case now normalises query (elastic#22431) ...

nik9000 added 2 commits January 24, 2017 16:52

Remove useless import

dbab668

nik9000 added :Plugin Lang Painless >enhancement v5.3.0 v6.0.0-alpha1 labels Jan 24, 2017

nik9000 requested review from clintongormley and jdconrad January 24, 2017 21:56

nik9000 commented Jan 24, 2017

View reviewed changes

clintongormley reviewed Jan 25, 2017

View reviewed changes

nik9000 added 2 commits January 25, 2017 13:51

Drop the link to class level documentation

4fe5fee

It is confusing and break screen readers.

Switch to definition list

e86e87c

jdconrad approved these changes Jan 25, 2017

View reviewed changes

Move api reference to appendix A and make visible in TOC

72aaae5

Fix test

674f3af

nik9000 mentioned this pull request Jan 25, 2017

Enhance painless documentation #22720

Closed

nik9000 merged commit 8a2d424 into elastic:master Jan 26, 2017

clintongormley added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache and removed :Plugin Lang Painless labels Feb 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate reference links for painless API #22775

Generate reference links for painless API #22775

nik9000 commented Jan 24, 2017 •

edited by clintongormley

Loading

nik9000 commented Jan 24, 2017

nik9000 Jan 24, 2017

nik9000 Jan 24, 2017

nik9000 Jan 24, 2017

nik9000 Jan 24, 2017

nik9000 Jan 24, 2017

nik9000 Jan 24, 2017

nik9000 Jan 24, 2017 •

edited

Loading

nik9000 Jan 24, 2017

nik9000 Jan 24, 2017

nik9000 commented Jan 24, 2017

clintongormley left a comment

clintongormley Jan 25, 2017

clintongormley Jan 25, 2017

nik9000 Jan 25, 2017

clintongormley Jan 25, 2017

nik9000 Jan 25, 2017

clintongormley Jan 25, 2017

nik9000 commented Jan 25, 2017

clintongormley commented Jan 25, 2017

nik9000 commented Jan 25, 2017

nik9000 commented Jan 25, 2017

jdconrad left a comment

nik9000 commented Jan 25, 2017

clintongormley commented Jan 26, 2017

nik9000 commented Jan 26, 2017

nik9000 commented Jan 26, 2017 •

edited

Loading

		@@ -0,0 +1,17 @@
		["appendix",role="exclude",id="painless-api-reference"]

Generate reference links for painless API #22775

Generate reference links for painless API #22775

Conversation

nik9000 commented Jan 24, 2017 • edited by clintongormley Loading

nik9000 commented Jan 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nik9000 Jan 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nik9000 commented Jan 24, 2017

clintongormley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nik9000 commented Jan 25, 2017

clintongormley commented Jan 25, 2017

nik9000 commented Jan 25, 2017

nik9000 commented Jan 25, 2017

jdconrad left a comment

Choose a reason for hiding this comment

nik9000 commented Jan 25, 2017

clintongormley commented Jan 26, 2017

nik9000 commented Jan 26, 2017

nik9000 commented Jan 26, 2017 • edited Loading

nik9000 commented Jan 24, 2017 •

edited by clintongormley

Loading

nik9000 Jan 24, 2017 •

edited

Loading

nik9000 commented Jan 26, 2017 •

edited

Loading