Skip to content

Conversation

wojtekmach
Copy link
Member

No description provided.

@josevalim josevalim changed the title Add :indent option to EEx Add :indentation option to EEx Jan 13, 2020
* `:file` - the file to be used in the template. Defaults to the given
file the template is read from or to "nofile" when compiling from a string.
* `:line` - the line to be used as the template start. Defaults to 1.
* `:indentation` - (since v1.11.0) indentation to be added to each column.
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems off to define an option using its name. Also I may be wrong, but I while internally it probably gets applied to each column in each line, I think lines are more important than columns here. How about:

Suggested change
* `:indentation` - (since v1.11.0) indentation to be added to each column.
* `:indentation` - (since v1.11.0) amount of spaces to be added before each line.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I'm not happy super happy with the wording either but fwiw we define e.g. :line in terms of line :)

My first thought when I read your proposed wording is as if we somehow literally inject space characters before each line and thus adjust output but the option is about adjusting :column in the AST so that e.g. stacktraces would point to the exact spot where we e.g. call a function. (again, not saying that's what you meant but how I'd likely have interpreted it.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, someone could interpret it this way. Not sure how to improve it then, it could stay as is until someone else has a better idea.

Copy link
Member

Choose a reason for hiding this comment

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

"an integer to be added to the column after every new line"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I went with:

    * `:indentation` - (since v1.11.0) an integer added to the column after every
       new line. Defaults to 0.

👍

@josevalim josevalim merged commit 94d9ea2 into elixir-lang:master Jan 14, 2020
@josevalim
Copy link
Member

❤️ 💚 💙 💛 💜

@wojtekmach wojtekmach deleted the wm-eex-indent branch February 1, 2020 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants