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: Keyboard shortcut to clean up entire object and combine with ABAP Formatter #42

Closed
ConjuringCoffee opened this issue Jun 2, 2023 · 8 comments
Assignees
Labels
discussion wanted enhancement New feature or request

Comments

@ConjuringCoffee
Copy link
Contributor

ConjuringCoffee commented Jun 2, 2023

Hi Jörg-Michael, my regular shortcut usage looks like this currently:

  • SHIFT+F1 to use the ABAP Formatter on the entire class
  • CTRL+A, then CTRL+SHIFT+4 to use the ABAP Cleaner on the entire class
  • CTRL+SHIFT+F3 to save and activate everything

That's quite some finger acrobatics. 😄 My request is for a keyboard shortcut to use the ABAP Cleaner on the entire class without having to select the entire class beforehand. (So essentially: Have a shortcut that uses the same selection mechanism as the ABAP Formatter.)

My second request is for a possibility to include the ABAP Formatter in the shortcut. The ABAP Cleaner doesn't fully replace the ABAP Formatter yet, so I still see the need to use it. (I can open a separate issue for this if you want to.)

In my ideal scenario I could then put this new shortcut on SHIFT+F1 and work like I used to.

@jelliottp
Copy link

Great suggestions here. We are almost always applying rules to a whole class, and most engineers are used to the ABAP formatter behaving this way already so it would be good to be consistent.

(I have also run into inconsistencies between the two like you mentioned, so I'll be interested to hear your feedback also. So far I have seen TEXT keyword as on example.)

@jmgrassau jmgrassau added enhancement New feature or request discussion wanted labels Jun 6, 2023
@jmgrassau
Copy link
Member

Hi ConjuringCoffee and @jelliottp,

just to be sure: the idea is to have an extra pair of shortcuts such as Ctrl+5 / Ctrl+Shift+5 (indeed those don't seem to be used in Eclipse yet), so the developer could control whether to clean the selection or current object (Ctrl+4) or the entire code document (Ctrl+5), right? So you don't mean a setting with which developers could configure the cleanup scope of Ctr+4 (e.g. "current object" / "entire code document") inside ABAP cleaner.

The request isn't new (it already came up during the Inner Source phase), but I always wondered whether blocking 4 shortcuts and adding 4 entries to the "Source Code" menu may be a bit (too) much?

Regarding ABAP Formatter, I don't think we could technically include it in the ABAP cleaner shortcut; however, I would be interested in examples of where ABAP cleaner doesn't replace ABAP Formatter yet!

Kind regards,
Jörg-Michael

@ConjuringCoffee
Copy link
Contributor Author

just to be sure: the idea is to have an extra pair of shortcuts such as Ctrl+5 / Ctrl+Shift+5 (indeed those don't seem to be used in Eclipse yet), so the developer could control whether to clean the selection or current object (Ctrl+4) or the entire code document (Ctrl+5), right? So you don't mean a setting with which developers could configure the cleanup scope of Ctr+4 (e.g. "current object" / "entire code document") inside ABAP cleaner.

Honestly, both would be fine for me. If you worry about blocking more shortcuts, then another possibility would be to provide the command without assigning a binding to it. There are lots of commands even in ADT that simply don't have a binding associated with them by default. (That doesn't solve the problem regarding too many entries in the "Source code" menu though.)

@ConjuringCoffee
Copy link
Contributor Author

Regarding ABAP Formatter, I don't think we could technically include it in the ABAP cleaner shortcut; however, I would be interested in examples of where ABAP cleaner doesn't replace ABAP Formatter yet!

I can collect the examples I find if you want. How do you want to receive them? In this issue or in a separate issue?

@jmgrassau jmgrassau self-assigned this Jun 11, 2023
@jmgrassau
Copy link
Member

Hi ConjuringCoffee and @jelliottp,

so here's what you will find in the next release: Next to the profile selection, the interactive UI now offers the new setting "Default cleanup range" (I reworked the whole section a bit):

image

"Default" means that if you select a specific range of code, ABAP cleaner will always only change this selected range. However, if the cursor is at some place inside the editor, but nothing is selected, ABAP cleaner will derive a "default cleanup range" from the cursor position. So far, ABAP cleaner used "Default cleanup range: Current method etc."; now you can change that into one of the following settings:

image

As with the profile, the last setting from the UI is also the new setting for automated cleanup – so if you set it to "Entire code document", Ctrl+4 will afterwards always do exactly that (until you change it again on the UI).

So, as long as you don't want to switch all the time, this might help :-) And as Josh wrote:

We are almost always applying rules to a whole class, and most engineers are used to the ABAP formatter behaving this way already

Kind regards,
Jörg-Michael

@ConjuringCoffee
Copy link
Contributor Author

ConjuringCoffee commented Jun 12, 2023

Cool, thank you. But what is the difference between Current class and Entire code document?

EDIT: Looks like Current class is relevant when working with multiple local classes. Knowing that, the naming makes sense and I don't know if there's a better way. Maybe the explanation could be added to the documentation?

@jmgrassau
Copy link
Member

Hi ConjuringCoffee and Josh,

hope this approach proves to be useful in practice! It is of course not as flexible as having two (or rather, four) shortcuts, but maybe those won't even be missed so much.

And thanks a lot, @ConjuringCoffee for question + answer + pull request on the meaning of "Current class"! (Will approve that in a minute…)

Kind regards,
Jörg-Michael

@ConjuringCoffee
Copy link
Contributor Author

Regarding ABAP Formatter, I don't think we could technically include it in the ABAP cleaner shortcut;

So.. in today's DSAG session it sounded like this could be possible after all? 😉

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

No branches or pull requests

3 participants