Bug 441448 - Give plugins implementing code formatters more freedom when dealing with empty selection
Summary: Give plugins implementing code formatters more freedom when dealing with empt...
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 4.4   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 4.5 M3   Edit
Assignee: Sergey Prigogin CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 419183 419399
  Show dependency tree
 
Reported: 2014-08-08 22:07 EDT by Sergey Prigogin CLA
Modified: 2014-10-28 05:08 EDT (History)
1 user (show)

See Also:
daniel_megert: review+

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Sergey Prigogin CLA 2014-08-08 22:07:48 EDT 
In order to implement a solution for bug 419399 jface.text framework has to give more freedom to plugins implementing code formatters when dealing with an empty selection. Currently SourceViewer class contains the following code that interprets an empty selection as formatting of the whole document. 

if (selection.y == 0) {
    context.setProperty(FormattingContextProperties.CONTEXT_DOCUMENT, Boolean.TRUE);
} else {
    context.setProperty(FormattingContextProperties.CONTEXT_DOCUMENT, Boolean.FALSE);
    context.setProperty(FormattingContextProperties.CONTEXT_REGION, new Region(selection.x, selection.y));
}

A proposed solution is to a allow the creator of the formatting context to indicate that an empty selection should be passed to the formatter instead of being replaced by the entire document.
Comment 1 Sergey Prigogin CLA 2014-08-09 01:25:46 EDT 
A proposed solution is in https://git.eclipse.org/r/#/c/31337/. The corresponding CDT change that addresses bug 419399 is in https://git.eclipse.org/r/#/c/31338/.
Comment 2 Sergey Prigogin CLA 2014-08-18 19:22:48 EDT 
Dani, could you please review the patch  https://git.eclipse.org/r/#/c/31337/ in Gerrit. Thanks you.
Comment 3 Dani Megert CLA 2014-09-15 04:41:33 EDT 
(In reply to Sergey Prigogin from comment #2)
> Dani, could you please review the patch 
> https://git.eclipse.org/r/#/c/31337/ in Gerrit. Thanks you.

Hi Sergey. Sorry, this got missed and I'm traveling this week. I'll look at this during M3 for sure.
Comment 4 Sergey Prigogin CLA 2014-10-09 13:39:12 EDT 
M3 is quickly approaching...
Comment 5 Sergey Prigogin CLA 2014-10-16 13:15:30 EDT 
(In reply to Dani Megert from comment #3)
> Hi Sergey. Sorry, this got missed and I'm traveling this week. I'll look at
> this during M3 for sure.

Hi Dani. In order to keep your promise you should probably review the patch now. Otherwise it is going to hit the code freeze week before M3.
Comment 6 Dani Megert CLA 2014-10-20 08:16:14 EDT 
(In reply to Sergey Prigogin from comment #5)
> (In reply to Dani Megert from comment #3)
> > Hi Sergey. Sorry, this got missed and I'm traveling this week. I'll look at
> > this during M3 for sure.
> 
> Hi Dani. In order to keep your promise you should probably review the patch
> now. Otherwise it is going to hit the code freeze week before M3.

It's definitely on my list for this week!
Comment 7 Dani Megert CLA 2014-10-21 09:23:36 EDT 
Hi Sergey, if added my comments in Gerrit.
Comment 9 Sergey Prigogin CLA 2014-10-27 23:06:15 EDT 
(In reply to Dani Megert from comment #8)
> Submitted with
> http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/
> ?id=396f7fa1c29c2318a8fc956bf7bdc836f9920f0c

For some mysterious reason this commit doesn't show up in http://git.eclipse.org/c/platform/eclipse.platform.text.git/log/
Comment 10 Dani Megert CLA 2014-10-28 05:08:37 EDT 
(In reply to Sergey Prigogin from comment #9)
> (In reply to Dani Megert from comment #8)
> > Submitted with
> > http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/
> > ?id=396f7fa1c29c2318a8fc956bf7bdc836f9920f0c
> 
> For some mysterious reason this commit doesn't show up in
> http://git.eclipse.org/c/platform/eclipse.platform.text.git/log/

Thanks for catching this. No idea what Gerrit did. Maybe it didn't merge because I just rebased before. Anyway, it's there now:
http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=1be15476ed99918ea8a9674d8ceb7884a430fc69