add option to skip outputting empty import lists in use statements#123
add option to skip outputting empty import lists in use statements#123ThomasLamprecht wants to merge 1 commit intoperl-ide:mainfrom
Conversation
This adds an option flag that, when enabled, causes perlimports to stop outputting empty import lists for use statements where there is no symbol to be imported. While some projects and people want empty import lists to increase clarity and prevent the used module from having default exports in the future, others see them as unnecessary visual noise and would like to avoid them. The CLI option is named `--skip-empty-imports`, and the configuration option is named `skip_empty_imports`. Both default to false, maintaining the current behavior that provides more deterministic clarity and future-proofs against export changes in the used modules. Thanks to the existing code infrastructure, the implementation is relatively straightforward. We just add new options and wire them up so that they are available in the Include module when outputting. Since there is already a dedicated code branch for modules that never export or from which nothing is imported, we can modify the output format string template there to differentiate between the respective values of the new skip_empty_imports flag. Add two tests to ensure that enabling and disabling explicitly work. Reuse the existing original-imports.pl test data module, as it is a fitting test asset. Closes: perl-ide#118 Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
|
Hmm, after talking a bit with a colleague in the office about this I got some doubts about semantic details. Namely, are modules that export by default, e.g., populate And if not so, should one prefer to unconditionally add empty import list for modules that export something by default, or should we allow the user to choose here by transforming the option into a tri-state enum? Both would not be much more code in total, so I think this is mostly a higher level question. I got a POC for a tri-state enum that makes the
At work, we would be content with the option staying boolean and the @mrsdizzie: maybe you can also chime in here given that you also commented on #118 |
|
@ThomasLamprecht I'm so sorry for how long this took. I won't bore you with the details. 😅 Thank you so much for this PR. Do you still have the branch with the tri-state enum handy? That sounds like a good solution. Barring that, I think we could still move forward with this. |
This adds an option flag that, when enabled, causes perlimports to stop outputting empty import lists for use statements where there is no symbol to be imported.
While some projects and people want empty import lists to increase clarity and prevent the used module from having default exports in the future, others see them as unnecessary visual noise and would like to avoid them.
The CLI option is named
--skip-empty-imports, and the configuration option is namedskip_empty_imports. Both default to false, maintaining the current behavior that provides more deterministic clarity and future-proofs against export changes in the used modules.Thanks to the existing code infrastructure, the implementation is relatively straightforward. We just add new options and wire them up so that they are available in the Include module when outputting. Since there is already a dedicated code branch for modules that never export or from which nothing is imported, we can modify the output format string template there to differentiate between the respective values of the new skip_empty_imports flag.
Add two tests to ensure that enabling and disabling explicitly work. Reuse the existing original-imports.pl test data module, as it is a fitting test asset.
Closes: #118
Open questions/tasks:
--range-beginand--range-endflags, and there the experimental status was only mentioned in the respective entry for the Changes file (ref commit e412e9c), so I opted for doing the same thing. Is that OK, or should I add other experimental markings somewhere?--skip-empty-importsand the config property nameskip_empty_importsOK as is?And FWICT from implementing this my available time should be more than enough to stick around for bug fixes, albeit–for full transparency–I might need a few days, and seldomly even one or two weeks to respond, depending on work crunch/time-off/etc.