Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Feature request: Un-align declarations #35

Closed
ConjuringCoffee opened this issue May 26, 2023 · 6 comments
Closed

Feature request: Un-align declarations #35

ConjuringCoffee opened this issue May 26, 2023 · 6 comments
Assignees

Comments

@ConjuringCoffee
Copy link
Contributor

ConjuringCoffee commented May 26, 2023

The Clean ABAP style guide suggests to not use chaining and not to align type declarations.

ABAP Cleaner provides the rule "Unchain into multiple statements", but any previous alignment is not removed. Could this be an option or another rule?

Example:

PARAMETERS:
  p_short  TYPE i,
  p_longer TYPE char20.

Result:

PARAMETERS p_short  TYPE i.
PARAMETERS p_longer TYPE char20.

Expected:

PARAMETERS p_short TYPE i.
PARAMETERS p_longer TYPE char20.
@fabianlupa
Copy link

Can you check if it's not automatically aligned after unchaining by the align declarations rule? You can see which rules applied in the preview window. I was also a bit surprised alignment across multiple statements is a feature here (#14 (comment)).

@ConjuringCoffee
Copy link
Contributor Author

I should have mentioned this: The rule "Align declarations" is inactive for these examples.

@jmgrassau
Copy link
Member

Hi ConjuringCoffee and Fabian,

generally, ABAP cleaner does NOT try to do everything within the same cleanup rule, so if you feed

PARAMETERS:
  p_short  type i  , ##PRAGMA_IN_WRONG_POSITION
  p_longer TYPE CHAR20  .

into the "Unchain into multiple statements" rule (using "Examples: Paste" in the profiles editor with activated "Unchain simple commands" option), you will get

PARAMETERS p_short  type i  . ##PRAGMA_IN_WRONG_POSITION
PARAMETERS p_longer TYPE CHAR20  .

So, upper/lower case is unchanged, spaces before comma / period are unchanged, pragma is still in wrong position, TYPE is still aligned.

So I think what you're actually missing is a cleanup rule to unalign declarations (unless they are chained). That's surely among the controversial topics (at least from my experience) … but admittedly, you're in line with the style guide here, while the "Align declarations" rule is not. That's why the "Align declarations" rule shows "<->" (indicating contradiction) in the references …

image

… and is inactive in the 'essential' profile.

So – you are basically asking for an option allowing to choose whether to a) align or b) unalign TYPE, right? – I guess adding this to the "Align declarations" rule would be clearer than having two contradicting rules ("Align declarations" and "Unalign declarations").

The other options such as "Align across empty lines" etc. could be grayed out if "unalign" is selected (cp. the "Auto-determine" option in the "Convert upper and lower case" rule).

Kind regards,
Jörg-Michael

@ConjuringCoffee
Copy link
Contributor Author

You got it right, Jörg-Michael. 👍

@ConjuringCoffee ConjuringCoffee changed the title Unchain into multiple statements: Remove alignment? Feature request: Un-align declarations May 26, 2023
@jmgrassau jmgrassau self-assigned this Jun 11, 2023
@jmgrassau
Copy link
Member

Hi ConjuringCoffee,

in the next release, the "Align declarations" rule will allow you to specify how much alignment you want:

image

You can select this independently for chains, non-chains, and structures (because Pretty Printer behaves differently for those, too). Everything that is not aligned can be condensed with the last option, thus removing spaces from any previous alignment.

So, with "align name, TYPE, LENGTH, VALUE etc." and activated "Condense …", you get:

    CONSTANTS: lc_any_price           TYPE ty_price           VALUE 1200,
               lc_other_price         TYPE ty_price           VALUE 1000,
               lc_num_contract_change TYPE ty_sequence_number VALUE 1,
               lc_start_date          TYPE ty_start_date      VALUE '20220312'.

    CONSTANTS lc_pi      TYPE p LENGTH 10 DECIMALS 10 VALUE '3.1415926536'.
    CONSTANTS lc_e       TYPE p LENGTH 8  DECIMALS 10 VALUE '2.718281828'.
    CONSTANTS lc_sqrt_2  TYPE p LENGTH 8  DECIMALS 4  VALUE '1.4142'.
    CONSTANTS lc_sqrt_32 TYPE p LENGTH 8  DECIMALS 10 VALUE '5.6568542495'.

With "align name and TYPE" and activated "Condense …":

    CONSTANTS: lc_any_price           TYPE ty_price VALUE 1200,
               lc_other_price         TYPE ty_price VALUE 1000,
               lc_num_contract_change TYPE ty_sequence_number VALUE 1,
               lc_start_date          TYPE ty_start_date VALUE '20220312'.

    CONSTANTS lc_pi      TYPE p LENGTH 10 DECIMALS 10 VALUE '3.1415926536'.
    CONSTANTS lc_e       TYPE p LENGTH 8 DECIMALS 10 VALUE '2.718281828'.
    CONSTANTS lc_sqrt_2  TYPE p LENGTH 8 DECIMALS 4 VALUE '1.4142'.
    CONSTANTS lc_sqrt_32 TYPE p LENGTH 8 DECIMALS 10 VALUE '5.6568542495'.

With "align name only" and activated "Condense …":

    CONSTANTS: lc_any_price TYPE ty_price VALUE 1200,
               lc_other_price TYPE ty_price VALUE 1000,
               lc_num_contract_change TYPE ty_sequence_number VALUE 1,
               lc_start_date TYPE ty_start_date VALUE '20220312'.

    CONSTANTS lc_pi TYPE p LENGTH 10 DECIMALS 10 VALUE '3.1415926536'.
    CONSTANTS lc_e TYPE p LENGTH 8 DECIMALS 10 VALUE '2.718281828'.
    CONSTANTS lc_sqrt_2 TYPE p LENGTH 8 DECIMALS 4 VALUE '1.4142'.
    CONSTANTS lc_sqrt_32 TYPE p LENGTH 8 DECIMALS 10 VALUE '5.6568542495'.

So the choice is now yours ;-)

Kind regards,
Jörg-Michael

@jmgrassau
Copy link
Member

Hi ConjuringCoffee and Fabian,

this is now available in release 1.4.0!

Kind regards,
Jörg-Michael

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants