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

Allowing nesting of variables #152

Open
Mpdreamz opened this issue Jan 8, 2025 · 7 comments
Open

Allowing nesting of variables #152

Mpdreamz opened this issue Jan 8, 2025 · 7 comments

Comments

@Mpdreamz
Copy link
Member

Mpdreamz commented Jan 8, 2025

Today the asciidoc system allows nesting of variables e.g:

:es:              Elasticsearch
:es-sql:          {es} SQL

:webhook:                 Webhook
:webhook-cm:              {webhook} - Case Management

:ml:                      machine learning
:ml-jobs:                    {ml} jobs

:es-repo:      https://github.com/elastic/elasticsearch/
:es-issue:     {es-repo}issues/
:es-pull:      {es-repo}pull/
:es-commit:    {es-repo}commit/

While ideally we don't take on this complexity it does seem very pervasively used.

Should we implement this or force a more simple key : value on to our writers and developers?

@bmorelli25 @KOTungseth @reakaleek

@reakaleek
Copy link
Member

reakaleek commented Jan 8, 2025

Superficial thought: I think it makes sense to keep it.

I encountered a similar use case in GitHub Actions with environment variables.

In GH Actions, you cannot define an environment variable and arleady use it on the same level.

Example:

I cannot do:

env:
  FIRST_NAME: Jan
  LAST_NAME: Calanog
  FULL_NAME: "${{ env.FIRST_NAME }} ${{ env.LAST_NAME }}"

What I have to do is:

env:
  FIRST_NAME: Jan
  LAST_NAME: Calanog
  FULL_NAME: Jan Calanog

I'm trying to say that being unable to nest it will cause duplication, and if you update a term, you need to update it in multiple places. This means you could forget to update some places or won't even find the term if they are written slightly differently.

But I understand that this also brings another set of complexities.

Similar to github actions though (which allows injecting envs in a lower level), we could at least support injecting global variables? this could be a balance between complexity and usefulness and enhance consistency throughout the whole documentation.

@reakaleek
Copy link
Member

reakaleek commented Jan 8, 2025

Not exactly about this issue. But I was thinking.. what do you think of giving global variables a distinct prefix, making it easily identifiable as global variable? e.g. _global_{varname} (just an example)

@Mpdreamz
Copy link
Member Author

Mpdreamz commented Jan 8, 2025

global{varname} (just an example)

I think for user defined variables (global/frontmatter) we should keep it original. If these clash the docs-builder will raise an error.

#151

As part of this issue I did suggest to introduce a prefix for predefined variables that docs-builder itself exposes.

@reakaleek
Copy link
Member

Suggestion:

We could introduce something like a convention in some guide or wherever.

Similar to programming language conventions, you usually write global variable in upper-snake-case (e.g. MY_GLOBAL_VAR)

@timto-elastic
Copy link

Are we ok with not having this expansion capability in the new system? Are there other ways to solve the problem(s)?

@timto-elastic
Copy link

Let's try to keep as simple as possible. Would this cause highl excessive writer pain or errors?

@bmorelli25
Copy link
Member

bmorelli25 commented Jan 9, 2025

I think nested variables make sense in the context of the use case Jan posted above—when two or more variables are being combined to create a longer variable:

env:
  FIRST_NAME: Jan
  LAST_NAME: Calanog
  FULL_NAME: "${{ env.FIRST_NAME }} ${{ env.LAST_NAME }}

I only see four instances of this in our attributes file. Here are a couple examples:

:es-security-features:    {es} {security-features}
:stack-security-features: {stack} {security-features}

For cases of a single attribute nested, I'm less convinced that this adds any value.

I'm trying to say that being unable to nest it will cause duplication, and if you update a term, you need to update it in multiple places. This means you could forget to update some places or won't even find the term if they are written slightly differently.

This is a potential risk.

Not exactly about this issue. But I was thinking.. what do you think of giving global variables a distinct prefix, making it easily identifiable as global variable? e.g. global{varname} (just an example)

Global variables should make up 95% of all vars in the system, so I don't want to add length to them. But, I don't hate the idea of prefixing local vars. They are used less frequently and it would be nice to have a distinction between them. Alternatively, I do also like your idea to have all global vars be in upper-snake-case.


I don't think we should support nested attrs for 9.0. Updating our existing attribute file to not use nested attributes is super easy. That unblocks writers for migration tasks. If this change causes pain for our writers (or missed updates in the attribute file) we can reassess in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants