Skip to content

Feature request (or bug?): Rule: "Standardize empty line within methods" should be able to eliminate (all) blank lines #131

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
JoachimRees opened this issue Oct 4, 2023 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@JoachimRees
Copy link

The rule "Standardize empty line within methods" should be able to eliminate all blank lines.
As of now, this (example from abap cleaner) is the shortest I seem to be able to get:

  METHOD empty_lines_within_methods.
    DATA lv_any_integer TYPE i.
    DATA lv_any_string  TYPE string.
    do_something( ).

    do_something_else( ).

    finish_doing_something( ).
  ENDMETHOD.

What I want ist this:

  METHOD empty_lines_within_methods.
    DATA lv_any_integer TYPE i.
    DATA lv_any_string  TYPE string.
    do_something( ).
    do_something_else( ).
    finish_doing_something( ).
  ENDMETHOD.

... Setting "Max empty lines within methods" to 0 might/would/should do exactly that.
But I cannot set it to 0 -> it's always set back to 1 :-( .
2023-10-04 16_13_05_ABAP_cleaner_Emty_Lines_0_not_Possible

@jmgrassau
Copy link
Member

Hi Joachim,

for each numeric input field, ABAP cleaner defines a range for possible input – you can try that by pressing Ctrl+Pos1 and Ctrl+End inside each of these fields (also Arrow Up/Down and Page Up/Down works).

In this case, entering "Max. empty lines within methods" = 0 would mean that inside methods, ABAP cleaner would remove all empty lines in all cases. I simply didn't expect that someone would want that, because wouldn't you agree that there can be good reasons to structure a method into sections with the help of empty lines? So there could be empty lines that were intentionally put there and you wouldn't want to remove them?

The styleguide says "Add a single blank line to separate things, but not more", and even in your short example, I'd personally prefer an empty line between the declarations and the executable statements.

But if you're sure you want to get rid of all empty lines, it would be very easy to allow a 0 here.

Kind regards,
Jörg-Michael

@JoachimRees
Copy link
Author

Hi Jörg-Michael,

I agree sometimes an empty line makes sense.
And I agree removing ALL sounds a bit extreme.

But: at least for method declarations, I really don't want empty lines between them. And I would love ABAP Cleaner to remove them for me.
So if it would be my call: do allow the 0! I'll give it a try with the next release and we see how that works out for me!

@JoachimRees
Copy link
Author

Oh, another thing:

The link you provided lists this anti-pattern:

" anti-pattern
METHOD do_something.

  do_this( ).

  then_that( ).

ENDMETHOD.

As far as I can tell, currently ABAP Cleaner can not fix that to:

METHOD do_something.
  do_this( ).
  then_that( ).
ENDMETHOD.

-> It could/would with the 0-option.

@jmgrassau
Copy link
Member

Hi Joachim,

sure, that's easily done:

image

With respect to the anti-pattern: What ABAP cleaner can do is remove the line breaks at the start and at the end of the method, because IMHO that always makes sense. It can also reduce multiple line breaks to just one. So, from this code…

METHOD do_something.

  do_this( ).


  then_that( ).

ENDMETHOD.

…you would then get:

METHOD do_something.
  do_this( ).

  then_that( ).
ENDMETHOD.

However, what ABAP cleaner can't do is decide whether the remaining line break is intentional and should therefore be kept. That would require a human being looking at what the code does and whether it helps readability to structure it into several sections with a blank line between them. (Well, you could introduce an option that says "if my method only consists of a maximum of [3] lines, then always remove all blank lines).

So, I guess this is a limitation of the approach ABAP cleaner has:

  • In ABAPQuickFix, you select a section of code, then the tool offers you the quick fixes available for that section, and then you explicitly select which fixes you want applied for that particular section. That's much more interaction, but it means that ABAPQuickFix can offer refactorings that make sense in some places, but wouldn't in others (such as "Remove all comments").
  • ABAP cleaner, on the other hand, once you've configured it, automatically executes all cleanup rules on all code, so you don't interact with it each time, saying "remove the empty lines here, but not there". With this, all is done in just one click, but on the other hand, ABAP cleaner must be able to automatically determine what to do, otherwise it can't do it.

Kind regards,
Jörg-Michael

@jmgrassau jmgrassau added the enhancement New feature or request label Oct 6, 2023
@jmgrassau jmgrassau self-assigned this Oct 6, 2023
jmgrassau added a commit to jmgrassau/abap-cleaner that referenced this issue Oct 10, 2023
@jmgrassau
Copy link
Member

Hi Joachim,

version 1.7.0 now allows to set this option to 0!

Kind regards,
Jörg-Michael

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants