Bug 419399 - Preference for Format with no selection
Summary: Preference for Format with no selection
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-editor (show other bugs)
Version: 8.2.1   Edit
Hardware: All All
: P3 enhancement with 3 votes (vote)
Target Milestone: 8.6.0   Edit
Assignee: Sergey Prigogin CLA
QA Contact: Anton Leherbauer CLA
URL:
Whiteboard:
Keywords:
Depends on: 441448
Blocks:
  Show dependency tree
 
Reported: 2013-10-14 16:44 EDT by Sergey Prigogin CLA
Modified: 2015-03-04 14:08 EST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sergey Prigogin CLA 2013-10-14 16:44:43 EDT
Current behavior of Ctrl-Shift-F with no selection is somewhat dangerous. If user hits Shift+Ctrl+F accidentally, for example by trying to search for references (Shift+Ctrl+G), the entire file is reformatted. In some cases the visible part of the file may not change and the user may continue to work being completely unaware of what happened. When the user later discovers effects of the accidental reformatting, it is already too late to use Undo since it would erase important changes made after the accident.

The problem can be remedied by introducing a preference for formatting without selection. The possible values for the preference are:
1. Format the entire file (current behavior)
2. Format the current statement
3. Do nothing (not sure if this option makes sense)

A similar feature request for JDT is in bug 419183.
Comment 1 Andrew Gvozdev CLA 2013-10-15 08:10:32 EDT
Perhaps asking a confirmation when the entire file is to be formatted could be a better solution?
Comment 2 Sergey Prigogin CLA 2013-10-15 14:03:20 EDT
(In reply to Andrew Gvozdev from comment #1)
> Perhaps asking a confirmation when the entire file is to be formatted could
> be a better solution?

I prefer the original proposal for two reasons:
1. Ability to format the current statement without having to select it is convenient.
2. Formatting the entire file takes two keystrokes - Ctrl+A, Shift+Ctrl+F. This is the same number of keystrokes as with a confirmation, but doesn't require user to wait for the dialog to appear before hitting the second keystroke.
Comment 3 Andrew Gvozdev CLA 2013-10-16 08:19:15 EDT
The dialog could provide an option "Do not ask again" which could be backed by the preference you are suggesting.
Comment 4 Sergey Prigogin CLA 2013-10-16 12:38:37 EDT
(In reply to Andrew Gvozdev from comment #3)
> The dialog could provide an option "Do not ask again" which could be backed
> by the preference you are suggesting.

Andrew, your proposal has been endorsed by Dani Megert in bug 419183 comment 10. I'll work on it.
Comment 5 Dani Megert CLA 2013-10-17 04:01:45 EDT
(In reply to Sergey Prigogin from comment #4)
> (In reply to Andrew Gvozdev from comment #3)
> > The dialog could provide an option "Do not ask again" which could be backed
> > by the preference you are suggesting.
> 
> Andrew, your proposal has been endorsed by Dani Megert in bug 419183 comment
> 10. I'll work on it.

Note that you will need to have two separate don't ask dialogs: one for the empty selection and one for the entire selection. Otherwise, users can't decide to say yes to one case and no to the other.

Would be great if you could also provide the same fix for JDT.
Comment 6 Sergey Prigogin CLA 2013-10-17 12:02:51 EDT
(In reply to Dani Megert from comment #5)
> Note that you will need to have two separate don't ask dialogs: one for the
> empty selection and one for the entire selection. Otherwise, users can't
> decide to say yes to one case and no to the other.

Selection of the entire file should be treated as any other non-empty selection and should not trigger a confirmation dialog.
Comment 7 Dani Megert CLA 2013-10-18 03:42:59 EDT
(In reply to Sergey Prigogin from comment #6)
> (In reply to Dani Megert from comment #5)
> > Note that you will need to have two separate don't ask dialogs: one for the
> > empty selection and one for the entire selection. Otherwise, users can't
> > decide to say yes to one case and no to the other.
> 
> Selection of the entire file should be treated as any other non-empty
> selection and should not trigger a confirmation dialog.

What's the difference then between having entire or no selection? In both cases you can accidentally format, and that's what you seem to be concerned about.
Comment 8 Sergey Prigogin CLA 2013-10-18 10:17:41 EDT
(In reply to Dani Megert from comment #7)
> What's the difference then between having entire or no selection? In both
> cases you can accidentally format, and that's what you seem to be concerned
> about.

I look at differently. The Format command operates on selection. Empty selection is a special case since it is treated as if the entire file was selected. It is logical to interpret the empty selection differently giving user a choice of the interpretation.
Comment 9 Dani Megert CLA 2013-10-18 10:20:07 EDT
(In reply to Sergey Prigogin from comment #8)
> (In reply to Dani Megert from comment #7)
> > What's the difference then between having entire or no selection? In both
> > cases you can accidentally format, and that's what you seem to be concerned
> > about.
> 
> I look at differently. The Format command operates on selection. Empty
> selection is a special case since it is treated as if the entire file was
> selected. It is logical to interpret the empty selection differently giving
> user a choice of the interpretation.

I can buy the *accidental* argument, but then making a difference whether all or nothing is selected sounds a bit strange.
Comment 10 Sergey Prigogin CLA 2013-10-18 12:02:29 EDT
(In reply to Dani Megert from comment #9)
> I can buy the *accidental* argument, but then making a difference whether
> all or nothing is selected sounds a bit strange.

If you look at the region subject to formatting as a function of the selection size, you can clearly see that the only discontinuity of this function is between selection of size 1 and an empty selection. Discontinuity is a special point by nature, so it is not strange that it deserves a special treatment.
Comment 11 Bart Samwel CLA 2014-01-20 09:21:12 EST
Ping on this. I just had to waste two hours untangling an accidental reformatting of a 2000-line file from the changes I'd made, and this hasn't been the first time by a long shot. This behavior may be OK for solitary developers, but in any situation where you have to submit clean patches for changes, this behavior is incredibly annoying. Thanks for your consideration!
Comment 12 Sergey Prigogin CLA 2014-08-09 01:28:41 EDT
A proposed solution is in https://git.eclipse.org/r/#/c/31338/. It depends on the change in platform text https://git.eclipse.org/r/#/c/31337/.
Comment 13 Nathan Ridge CLA 2014-08-12 00:34:00 EDT
(In reply to Dani Megert from comment #9)
> (In reply to Sergey Prigogin from comment #8)
> > (In reply to Dani Megert from comment #7)
> > > What's the difference then between having entire or no selection? In both
> > > cases you can accidentally format, and that's what you seem to be concerned
> > > about.
> > 
> > I look at differently. The Format command operates on selection. Empty
> > selection is a special case since it is treated as if the entire file was
> > selected. It is logical to interpret the empty selection differently giving
> > user a choice of the interpretation.
> 
> I can buy the *accidental* argument, but then making a difference whether
> all or nothing is selected sounds a bit strange.

The difference is that selecting the entire file contents requires you to _do_ something, and it's clear from the UI (every line being highlighted) that the next action will apply to the entire contents.
Comment 14 Sergey Prigogin CLA 2014-10-28 23:26:30 EDT
Fixed by commits http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=27f00a30d886dc781df348ea1cc8bd5f228c42ca and http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=08fd13ed9f55297f738363484912acd806b8cbde

Although the functionality is available in CDT 8.6, it manifests itself only on Platform 4.5M3 or higher.
Comment 15 CDT Genie CLA 2014-10-29 01:00:12 EDT
*** cdt git genie on behalf of Sergey Prigogin ***

    Bug 419399 - Preference for Format with no selection
    A minor fix in the preference page.

[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=08fd13ed9f55297f738363484912acd806b8cbde
Comment 16 Marc Khouzam CLA 2014-10-29 09:13:21 EDT
Is this worth a mention in the N&N?
https://wiki.eclipse.org/CDT/User/NewIn86
Comment 17 Sergey Prigogin CLA 2014-10-29 10:19:56 EDT
(In reply to Marc Khouzam from comment #16)
> Is this worth a mention in the N&N?
> https://wiki.eclipse.org/CDT/User/NewIn86

Since it requires Platform 4.5 that will be released in June, it makes sense to describe it in https://wiki.eclipse.org/CDT/User/NewIn87
Comment 18 Marc Khouzam CLA 2014-10-29 11:22:13 EDT
(In reply to Sergey Prigogin from comment #17)

> Since it requires Platform 4.5 that will be released in June, it makes sense
> to describe it in https://wiki.eclipse.org/CDT/User/NewIn87

Good point.
I've created the page for you.  I figure it is easier to add it now when it is fresh in your mind and environment than later on.

https://wiki.eclipse.org/CDT/User/NewIn87
Comment 19 Sergey Prigogin CLA 2014-10-29 13:05:45 EDT
(In reply to Marc Khouzam from comment #18)
Thanks a lot!
Comment 20 Marc Khouzam CLA 2014-10-29 13:08:22 EDT
(In reply to Marc Khouzam from comment #18)

> Good point.
> I've created the page for you.  I figure it is easier to add it now when it
> is fresh in your mind and environment than later on.
> 
> https://wiki.eclipse.org/CDT/User/NewIn87

(In reply to Sergey Prigogin from comment #19)
> (In reply to Marc Khouzam from comment #18)
> Thanks a lot!

Well, I created the page but didn't put an entry for this new change. So let me thank you in advance for putting that in :)
Comment 21 CDT Genie CLA 2014-11-01 01:00:35 EDT
*** cdt git genie on behalf of Sergey Prigogin ***

    Bug 419399 - Preference for Format with no selection
    More robust implementation of region expansion.

[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=90e95ab4eb6c4aa06fd1b894255582964631e533
Comment 22 Eclipse Genie CLA 2015-03-04 14:08:45 EST
New Gerrit change created: https://git.eclipse.org/r/43181