eclipse.platform.ui icon indicating copy to clipboard operation
eclipse.platform.ui copied to clipboard

Added "Close mode" frame in editor preference

Open praveen-skp opened this issue 1 year ago • 11 comments

Issue: #2297

This pull request adds a "Close mode" frame in the editor preference to make the "Close editors automatically" and "Number of opened editors before closing" grouping clear.

praveen-skp avatar Sep 19 '24 09:09 praveen-skp

Close mode

The new UI with "Close mode" grouping

praveen-skp avatar Sep 19 '24 09:09 praveen-skp

Close mode as a group name reads a bit strange here, I would expect then having different modes to select but the configuration do not really offer that.

My suggestion would be to merge both settings into one line instead of grouping them e.g.

[ ] Close oldest if there are more than _____ editors open

laeubi avatar Sep 19 '24 09:09 laeubi

Test Results

 1 818 files  + 1   1 818 suites  +1   1h 33m 19s :stopwatch: - 6m 3s  7 709 tests ± 0   7 481 :white_check_mark: + 1  228 :zzz: ± 0  0 :x:  - 1  24 288 runs  +45  23 541 :white_check_mark: +36  747 :zzz: +10  0 :x:  - 1 

Results for commit 263f51cb. ± Comparison against base commit a0221165.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Sep 19 '24 10:09 github-actions[bot]

Close mode as a group name reads a bit strange here

What about "Closing behaviour"?

BeckerWdf avatar Sep 19 '24 10:09 BeckerWdf

My suggestion would be to merge both settings into one line instead of grouping them

This is also a good idea.

BeckerWdf avatar Sep 19 '24 10:09 BeckerWdf

This is how the new UI looks. Disabled Enabled

praveen-skp avatar Oct 04 '24 05:10 praveen-skp

Can the input field made shorter so that the line doesn't get too long. I think place for around 4 chars would be sufficient.

What about changing the text to: "Close oldest editor if there more then ... editor open"

What does "oldest" mean? a) The one that was open the longest time, or b) the one where the time of last use is most far in the past?

BeckerWdf avatar Oct 04 '24 06:10 BeckerWdf

I checked the current behaviour and noticed that it's not always the oldest editors that are being closed.

Use case 1:

If I have the following editors opened in order: 1, 2, 3, 4, 5, 6 (1 being the oldest, focus is on 6)

  • Set editor limit to 3
  • Open editor 7
  • The current available editors are 7, 2, 3

It deletes the latest 3 editors (6, 5, 4) and replaces the oldest editor.


Use case 2:

If I have the following editors opened in order: 1, 2, 3, 4, 5, 6 (1 being the oldest, focus is on 5)

  • Set editor limit to 3
  • Open editor 7
  • The current available editors are 7, 2, 3

It deletes the latest 3 editors (5, 6, 4) and replaces the oldest editor (1).


Use case 3:

If I have the following editors opened in order: 1, 2, 3, 4, 5, 6 (1 being the oldest, focus is on 2)

  • Set editor limit to 3
  • Open editor 7
  • The current available editors are 7, 3, 4

It deletes the latest 3 editors (2, 6, 5) and replaces the oldest editor (1).


Use case 4:

If I have the following editors opened in order: 1, 2, 3 (1 being the oldest, focus is on 3)

  • Set editor limit to 3
  • Open editor 4
  • The current available editors are 4, 2, 3

It replaces the oldest editor (1).


Use case 5:

If I have the following editors opened in order: 1, 2, 3 (1 being the oldest, focus is on 1)

  • Set editor limit to 3
  • Open editor 4
  • The current available editors are 1, 4, 3

It replaces the oldest editor (2).

praveen-skp avatar Oct 08 '24 03:10 praveen-skp

It looks like its not always the oldest editors that are being closed. So shall we change the text to something generic like: Close editor(s) if there are more than ... editors open

praveen-skp avatar Oct 08 '24 03:10 praveen-skp

Close editor(s) if there are more than ... editors open

yes.

BeckerWdf avatar Oct 08 '24 09:10 BeckerWdf

Close editor(s) if there are more than ... editors open

yes. Maybe without the () simply "Close editors if..."

BeckerWdf avatar Oct 08 '24 09:10 BeckerWdf

Reduced size

Reduced the size of the input field to show two characters, to match the text limit of 2 characters in the code.

praveen-skp avatar Oct 09 '24 03:10 praveen-skp

merge commit are not allowed in PR. Can you pls. squash and rebase on top of current master please?

BeckerWdf avatar Oct 10 '24 08:10 BeckerWdf