[DOCS] Reformat condition token filter#48775
[DOCS] Reformat condition token filter#48775jrodewig merged 5 commits intoelastic:masterfrom jrodewig:reformat.condition-tokenfilter
Conversation
|
Pinging @elastic/es-search (:Search/Analysis) |
|
Pinging @elastic/es-docs (>docs) |
| token. | ||
|
|
||
| Only inline scripts are supported. For valid parameters, see | ||
| <<_script_parameters>>. |
There was a problem hiding this comment.
I'm not entirely sure where this links to, did a quick grep in the es repo and couldn't dig it up but I suppose its a general painless script page. When looking at the code about which kind of scripts are allowed here I was suprised to see that this is a specific kind of script (in the code called AnalysisPredicateScript) that seems to be limited to work on certain token attributes (like length, position etc...) only. Maybe its worth mentioning that this only works on token properties (so looks like "token.*" would always be the first part in the chain). @jdconrad should be able to validate this assumption.
There was a problem hiding this comment.
Thanks @cbuescher. For clarity, this currently links here:
https://www.elastic.co/guide/en/elasticsearch/reference/master/modules-scripting-using.html#_script_parameters
That content covers generic parameters for the script object. They aren't specific to Painless.
I think that link is still probably the best place to get those generic parameters, but I can also link to this page for Painless scripts used in this parameter:
https://www.elastic.co/guide/en/elasticsearch/painless/current/painless-analysis-predicate-context.html
Would that work for you @cbuescher @jdconrad? If only Painless scripts are allowed here, I can try another approach.
There was a problem hiding this comment.
Script contexts are supposed to be generic, so I would prefer to try to document them as such. I don't have a strong opinion one way or another on a link to Painless docs.
| "type": "condition", | ||
| "filter": [ "reverse" ], | ||
| "script": { | ||
| "source": "token.getPosition() === 0" |
There was a problem hiding this comment.
Maybe add a short comment after this example explaining what this analyzer does, its pretty obvious from the analyzer name but for some folks it might be worth spelling it out.
There was a problem hiding this comment.
Thank you for the comment! I was debating whether that was needed.
I added a description to the preceding paragraph with 3653233.
jdconrad
left a comment
There was a problem hiding this comment.
These changes look good to me. Thanks for making them. Moving forward, I would like to talk about the best way to document script contexts in general. @rjernst and I had some thoughts on how we might be able to automate some of this.
|
Thanks for your review @jdconrad. I agree that the documentation for script contexts could be improved, at least in how it's surfaced to users. I'd love to hear more about the ideas for automation. |
cbuescher
left a comment
There was a problem hiding this comment.
Thanks for the clarification and the short explanation of the example, great read as always!
Reformats the condition token filter docs as part of #44726.
Changes