Skip to content

indent filter can indent with arbitrary characters#1167

Merged
davidism merged 1 commit intopallets:masterfrom
LarsKollstedt:allow_indent_with_generic_char
Apr 5, 2021
Merged

indent filter can indent with arbitrary characters#1167
davidism merged 1 commit intopallets:masterfrom
LarsKollstedt:allow_indent_with_generic_char

Conversation

@LarsKollstedt
Copy link
Copy Markdown
Contributor

Implemenented also indention with tabulator characters instead of
whitespace characters. This will also allow the use of other characters or character sequences. It's the more generic way I already promised in #1166, choose the best and close the other one.

This is needed if this is used within a template with a specified tab indention. ;-)

Kind regards
Lars

@davidism
Copy link
Copy Markdown
Member

Let's do what json.dump does. If width is a string, it's used as the indent, otherwise it's the number of spaces to use as the indent. Makes it a bit weird to call it width, but I don't want to deal with a deprecation cycle just to rename it.

@davidism davidism changed the title filters.py: do_indent: generic indent character indent filter can indent with arbitrary characters Apr 12, 2020
@LarsKollstedt LarsKollstedt force-pushed the allow_indent_with_generic_char branch from 08db4a3 to 520a578 Compare April 14, 2020 07:51
LarsKollstedt added a commit to LarsKollstedt/jinja that referenced this pull request Apr 14, 2020
Allow indention with generic characters, e.g. Tabs.

Implemenented the behavior requested in
pallets#1167 (comment)

The width param can be string or int, if it is string it's the raw
indention e.g. "\t". If it's int it's the number of spaces. In other
cases an FilterArgumentError is raised, to avoid confusion.
LarsKollstedt added a commit to LarsKollstedt/jinja that referenced this pull request Apr 14, 2020
Allow indention with generic characters, e.g. Tabs.

Implemenented the behavior requested in
pallets#1167 (comment)

The width param can be string or int, if it is string it's the raw
indention e.g. "\t". If it's int it's the number of spaces. In other
cases an FilterArgumentError is raised, to avoid confusion.
@LarsKollstedt LarsKollstedt force-pushed the allow_indent_with_generic_char branch from 520a578 to 36e289e Compare April 14, 2020 07:56
LarsKollstedt added a commit to LarsKollstedt/jinja that referenced this pull request Apr 14, 2020
Allow indention with generic characters, e.g. Tabs.

Implemenented the behavior requested in
pallets#1167 (comment)

The width param can be string or int, if it is string it's the raw
indention e.g. "\t". If it's int it's the number of spaces. In other
cases an FilterArgumentError is raised, to avoid confusion.
@LarsKollstedt LarsKollstedt force-pushed the allow_indent_with_generic_char branch from 36e289e to 188e945 Compare April 14, 2020 08:02
LarsKollstedt added a commit to LarsKollstedt/jinja that referenced this pull request Apr 14, 2020
Allow indention with generic characters, e.g. Tabs.

Implemenented the behavior requested in
pallets#1167 (comment)

The width param can be string or int, if it is string it's the raw
indention e.g. "\t". If it's int it's the number of spaces. In other
cases an FilterArgumentError is raised, to avoid confusion.
@LarsKollstedt LarsKollstedt force-pushed the allow_indent_with_generic_char branch from 188e945 to e423f78 Compare April 14, 2020 08:07
LarsKollstedt added a commit to LarsKollstedt/jinja that referenced this pull request Apr 14, 2020
Allow indention with generic characters, e.g. Tabs.

Implemenented the behavior requested in
pallets#1167 (comment)

The width param can be string or int, if it is string it's the raw
indention e.g. "\t". If it's int it's the number of spaces. In other
cases an FilterArgumentError is raised, to avoid confusion.
@LarsKollstedt LarsKollstedt force-pushed the allow_indent_with_generic_char branch from e423f78 to c2d9393 Compare April 14, 2020 08:19
@LarsKollstedt
Copy link
Copy Markdown
Contributor Author

LarsKollstedt commented Apr 14, 2020

Implemented it as requested in #1167 (comment). The reuse of 'width' does'nt look like the finest solution to me, but it fit's my needs.

@matejak
Copy link
Copy Markdown

matejak commented Aug 14, 2020

Why not to introduce an indent_with kwarg indent_char kwarg that would default to single space, to decouple the level of indentation from indentation type?

@LarsKollstedt
Copy link
Copy Markdown
Contributor Author

The original function has an attribute width that can only contain a length. My first suggestion for this was 08db4a3 which was introducing a additional param called indent_char, which can of course also contain something like " ". The width attribute was still conaining only a length there, and the indention type was decoupled.
I changed this due to #1167 (comment), but @davidism thought also of renaming the param to indent, but discarded this because of legacy reasons. Since this params are optional, theoretically you can have both a param indent that contains the whole indention, and a param width that contains the length with a param indent_char, that contains a single indention sequence. But I think that will cause people to discuss about deprecating some of them.
To rename the length parameter width to indent_with and to introduce a new width parameter with the indention type, will be one of the worst of possible all options.

@matejak
Copy link
Copy Markdown

matejak commented Aug 18, 2020

To rename the length parameter width to indent_with and to introduce a new width parameter with the indention type, will be one of the worst of possible all options.

Sorry, I indeed wrote a nonsense. What I meant was to accompany width a kwarg indent_string or indent_char that would default to space. Arguments indent, width and indent_char are not orthogonal, so I also wouldn't support an interface that uses all three in one macro.

@davidism
Copy link
Copy Markdown
Member

Let's do what json.dump does.

@davidism davidism added this to the 3.0.0 milestone Mar 27, 2021
Allow indention with generic characters, e.g. Tabs.

Implemenented the behavior requested in
pallets#1167 (comment)

The width param can be string or int, if it is string it's the raw
indention e.g. "\t". If it's int it's the number of spaces. In other
cases an FilterArgumentError is raised, to avoid confusion.
@davidism davidism force-pushed the allow_indent_with_generic_char branch from c2d9393 to 8016b5f Compare April 5, 2021 16:51
@davidism davidism merged commit 1b874db into pallets:master Apr 5, 2021
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Apr 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants