Could we add a mode similar to 0/grid mode, but that uses the fixed indent width for subsequent lines? Mode 0 looks like this:
from third_party import (lib1, lib2, lib3,
lib4, lib5, ...)
I would like this (where indent=2):
from third_party import (lib1, lib2, lib3,
lib4, lib5, ...)
This would be more compact, and can reduce line count. I would be happy to attempt a PR; pointers on where to look would be welcome. isort has been a great improvement to my workflow; thank you for building it!
FYI, the current mode 0 matches the pep8 style guide whereas the suggestion for indent does not.
While I don't have the bandwith to add support for this requested feature myself, I will accept a pull request that adds it. The correct place to add this functionality is here: https://github.com/timothycrosley/isort/blob/develop/isort/wrap_modes.py#L46
Thanks @timothycrosley. I'll try to find time to take a stab at this. Line 36 says that name and order are significant; should I add a new @_wrap_mode decorated function to the end, after noqa?
@gwk,
Glad to hear! Look forward to the pull request. Yes that's correct, right after noqa.
Thanks!
~Timothy
Hi there,
I looked into this one a bit and it can be almost achieved with the HANGING_INDENT or 2 mode. The only problem is that HANGING_INDENT hard-codes the usage of \ and ignores the --use-parentheses completely.
This problem seems to occur in the other modes as well, but the other way around ie. most of them hard-code the usage of parentheses.
I'm guessing that people don't notice this, because they probably use a profile and the profiles tend to use use_parentheses: True or they just prefer using parentheses. However, turning off use_parentheses isn't possible for most modes.
I made some modifications in my fork that makes HANGING_INDENT mode "respect" the --up option.
With this change, and an indent of 2, you can achieve what the OP wanted:
$isort -d test.py -m HANGING_INDENT -i 2 --up
from third_party import (lib1, lib2, lib3, lib4, lib5, lib6, lib7, lib8, lib9,
lib10, lib11, lib12, lix1, lix2, lix3, lix4, lix5, lix6, lix7, lix8, lix9,
lix10, lix11, lix12)
Please let me know what you think about this approach.
https://github.com/sztamas/isort/commit/c74041ec363098f398f7539fa02643424edb4df1
If it is OK, I would prefer to look at the tests covering this area and add some additional ones and/or update some to reflect the changes.
It is also probably a good idea to update the other modes to use backslash-joining by default and use parentheses only if use_parentheses is True. Would you agree?
Maybe that could be done as a separate issue and PR?
Thanks,
Tamas
This looks like what I was hoping for. My only question is, does this insert parentheses unconditionally or only when the line needs to wrap?
Will put more details here later, but, the use_parenthesis is not meant for wrap modes but only meant for single line wrapping with one import. isort has a release gaurentee that the same options will return the same output, other than fixing large errors. We could think about changing that in the future for 6.x.x but for the meantime code can be refactored to reuse logic, but a different wrap mode rather then utilizing the --up would be needed for this ticket
Some additional information:
Here is the official release policy: https://timothycrosley.github.io/isort/docs/major_releases/release_policy/#formatting-guarantees
Here is the updated documentation on the use_parentheses option to clarify its intended and effective usage: https://timothycrosley.github.io/isort/docs/configuration/options/#use-parentheses
If you look at where it's used, it only gets used for individual line wrapping, but never effects wrap modes: https://github.com/timothycrosley/isort/search?q=use_parentheses&unscoped_q=use_parentheses. I'd also note that profiles are very new, but this behaviour has existed for a few years now.
Thanks!
~Timothy
Hi @timothycrosley
All that makes a lot of sense.
What would be a good name for the new wrap mode?
I assume it will have to be added as the last mode to keep things backward compatible.
As a side note, it might be just me, but the mode names don't really tell me what the difference is between them.
Wouldn't it make sense to have a generated doc page when you show the modes with some examples to see what the difference is without having to try them out one-by-one?
Thanks,
Tamas
This looks like what I was hoping for. My only question is, does this insert parentheses unconditionally or only when the line needs to wrap?
Hi @gwk
It inserts the parentheses just when the lines wraps.
Thanks,
Tamas
What would be a good name for the new wrap mode?
Open to any name that makes sense, your initial observations make me think HANGING_INDENT_WITH_PARENTHESES might be a good candidate - though it is very verbose.
I assume it will have to be added as the last mode to keep things backward compatible.
Yes that's correct! If that assumption isn't commented in the source file via comments and/or via tests, another improvement that could be made is clarifying it.
As a side note, it might be just me, but the mode names don't really tell me what the difference is between them.
Wouldn't it make sense to have a generated doc page when you show the modes with some examples to see what the difference is without having to try them out one-by-one?
:100: % This. It would be really nice to have autogenerated docs, under the config tab of the new documentation website. Maybe pick a long example multi import statement and auto run it through all modes. Again this is just a bonus improvement that could be done.
Thanks for all the thought put into this!
~Timothy
I like HANGING_INDENT_WITH_PARENTHESES and although is long, there is precedent, it comes only as shared :2nd_place_medal: :smile:
VERTICAL_PREFIX_FROM_MODULE_IMPORT
VERTICAL_HANGING_INDENT_BRACKET
HANGING_INDENT_WITH_PARENTHESES
VERTICAL_GRID_GROUPED_NO_COMMA
VERTICAL_HANGING_INDENT
...
We have the numeric short-hands OFC, maybe worth introducing short-hands based on the first character of the modes that have more than 1 word? Would be similar to what you have for normal options already, backward compatible and there are no conflicts ATM:
0, GRID
1, VERTICAL
2, HANGING_INDENT, HI
3, VERTICAL_HANGING_INDENT, VHI
4, VERTICAL_GRID, VG
5, VERTICAL_GRID_GROUPED, VGG
6, VERTICAL_GRID_GROUPED_NO_COMMA, VGGNC
7, NOQA
8, VERTICAL_HANGING_INDENT_BRACKET, VHIB
9, VERTICAL_PREFIX_FROM_MODULE_IMPORT, VPFMI
10, HANGING_INDENT_WITH_PARENTHESES, HIWP
That's another idea that can result in another issue/PR though :smile: , going back to this issue there is still one related problem left.
The current hanging indent mode doesn't take --tc into consideration. That must be a bug!
Am I fixing it while I'm at it or would you prefer that to be separated into another separate issue?
Thanks,
Tamas
OK, I think this is done in the PR linked above.
Usage:
$ isort -d test.py -m HANGING_INDENT_WITH_PARENTHESES -i2
from third_party import (lib1, lib2, lib3, lib4, lib5, lib6, lib7, lib8, lib9,
lib10, lib11, lib12, lib13, lib14, lib15, lib16, lib17, lib18, lib19, lib20,
lib21, lib22, lib23, lib24, lib25, lib26, lib27, lib28, lib29, lib30)
I think wrap_modes.py could be refactored a bit, to remove some of the duplication and make the intent a bit clearer, but I refrained, in order to get the new feature in.
This was just deployed with version 5.2.0 of isort (thanks @sztamas!).
~Timothy
@sztamas & @timothycrosley, I just want to say thank you for implementing this. I finally got back to working on my library and this did the trick!
Most helpful comment
FYI, the current mode 0 matches the pep8 style guide whereas the suggestion for indent does not.