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

[DOCS] Add space to fix Asciidoctor output #41579

Merged
merged 1 commit into from
Apr 26, 2019
Merged

[DOCS] Add space to fix Asciidoctor output #41579

merged 1 commit into from
Apr 26, 2019

Conversation

jrodewig
Copy link
Contributor

@jrodewig jrodewig commented Apr 26, 2019

Asciidoctor outputs ((MIN(salary) / 100)) as <indexterm><primary>MIN(salary) / 100</primary></indexterm>. This adds whitespace to correct the output in preparation for the Asciidoctor migration.

Relates to elastic/docs#827 and #41128.

@@ -607,7 +607,7 @@ M |57
groupByAndAggExpression
// tag::groupByAndAggExpression
schema::g:s|salary:i
SELECT gender AS g, ROUND((MIN(salary) / 100)) AS salary FROM emp GROUP BY gender;
SELECT gender AS g, ROUND( (MIN(salary) / 100) ) AS salary FROM emp GROUP BY gender;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is in the file called docs.csv-spec but I'm worried that folks will remove the spacing without realizing the change that it makes in the docs. I wonder if there is a better way to change this. Let me look around some!

@jrodewig jrodewig added :Docs >docs General docs changes labels Apr 26, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs

@nik9000
Copy link
Member

nik9000 commented Apr 26, 2019

This failure is caused because we use the macros substitution in this snippet which is required by AsciiDoc because include-tagged is a macro. But in Asciidoctor it isn't a macro. All include things are "special" and on regardless. So one way to fix this is:

+ifdef::asciidoctor[]
+[source,sql]
+endif::[]
+ifndef::asciidoctor[]
 ["source","sql",subs="attributes,callouts,macros"]
+endif::[]

This suggests that we probably should remove ,subs="attributes,callouts,macros" across the board once we migrate to asciidoctor as it is liable to cause trouble like these indexterms.

Now that I understand it I'm fine with either fix so long as we make sure to blast ,subs="attributes,callouts,macros" on the modern branches once we've migrated....

@jrodewig
Copy link
Contributor Author

Thanks again, @nik9000! That asciidoctor conditional could be very handy.

I created #41589 to remove the subs="attributes,callouts,macros" options after Asciidoctor migration. That should keep us safe if that space is later removed.

@jrodewig jrodewig merged commit 32be700 into elastic:master Apr 26, 2019
@jrodewig jrodewig deleted the asciidoctor-sql-syntax-select branch April 26, 2019 16:14
@jaymode jaymode added v7.0.1 and removed v7.0.2 labels Apr 29, 2019
akhil10x5 pushed a commit to akhil10x5/elasticsearch that referenced this pull request May 2, 2019
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants