Community
Participate
Working Groups
Improve C/C++ editor to support "smart" indenting similar to Java editor.
Created attachment 45274 [details] Smart indening implementation for C editor
I don't quite understand what this patch should accomplish. It just adds a lot of formatter options (presumably cloned from JDT), but does not implement the actual formatting/indenting according to those options. Maybe the patch is not complete?
The patch implements smart indenting and automatic closing of braces. To see the difference try to press Enter at different places in C++ code with and without the patch. Try for example: if (first_part_of_a_condition && second_part_of_a_condition) {<------ Caret position The patch does not have UI for changing indenting options and does not automatically close parentheses and quotes. Even with these limitations this patch is a significant improvement over existing CEditor indentation policy. I will add more smart indenting functionality and UI for changing indenting preferences after this patch is applied. You are right that most of the code is cloned from JDT. I copied most Java formatting options over since they are likely to be needed soon. Their presence in the code does not hurt since they are not exposed in UI yet.
I noticed that the comments for the formatter option ids still have the jdt like path. I suggest that the ids in the comments be updated before applying the patch.
Yes, I noticed that, too and I don't like it, but above all I still can't see what difference the patch should make. Maybe I am completely dumb, but I get the same results with or without it. I am quite sure the patch is incomplete as it contains only changes to the core plug-in and no changes to ui. Could you - fix references to JDT and Java in comments and code (note also the @since tags) - remove or comment out unused / unimplemented formatting options (I don't think we would need or support them all too soon) - create a complete patch - add a description of what it does ("smart" is a bit unspecific) And, of course, unit tests would be great (see CAutoIndentTest). Thank you!
> without the patch. Try for example: > if (first_part_of_a_condition && > second_part_of_a_condition) {<------ Caret position Does not work for me (either).
Created attachment 46342 [details] Smart identing for C editor with a test.
I do apologize for the previous version of the patch that somehow was missing cdt.ui. The new version was tested by applying it to a virgin code. The patch implements a subset of functionality that in JDT is called "smart indenting". It puts caret in proper position for typing a new line of code after pressing Enter and closes an open brace when appropriate. The UI for changing code formatting preferences will be added in a separate patch. Comments are cleaned from references to jdt and java. @since tags are removed. Unused formatting options are commented out. CAutoIndentTest is updated to test new indenting behavior. While testing this patch I discovered a scenario that is not covered by the fix for bug 48339. Create a new .cc file. Type: /* */ Press Backspace, then Enter. The cursor moves to a new line but " *" is not inserted.
Created attachment 46343 [details] Cleaned a corrupted copyright comment
I applied your patch after consolidating the two different ICPartitions interfaces into one. Now there is only org.eclipse.cdt.ui.text.ICPartitions. > I will add more smart indenting functionality and UI for changing indenting > preferences after this patch is applied. Preference options are really necessary. I am looking forward to your contributions!
Thanks a lot!
Created attachment 47827 [details] More "smart editing" features and preferences UI This pretty big patch introduced more "smart editing" functionality including auto closing of strings, parentheses and brackets, auto escaping when pasting into strings and auto indenting of pasted code. It also introduces preferences UI for all new features plus the previously added smart indenting and closing of braces. I was not sure what to do with a defunct Code Formatter preference page. I ended up incorporating its functionality into new Code Style page. The code formatter pull down is only visible when there is anything to choose from. org.eclipse.cdt.internal.ui.preferences.formatter package puts a foundation for code formatter preferences UI. Right now only the indentation page is used.
Just a note for anyone planning on committing this patch to make sure that you run it through the IP check process first. It passes the size criteria. Thanks.
(In reply to comment #12) > Created an attachment (id=47827) [edit] Sergey, I tried some of the features you advertised and mostly they are working fine. Hey, makes me feel Java-ish in the C editor. Thanks a lot! Here are a few drawbacks I noticed: * When you are about it, why don't you make it so that for a new case statement in a switch the break is added automatically, similar to auto-closing braces, brackets and stuff? * case-statements get unindented alright when writing a switch, but the default: does not. It should be unindented like the cases. * When writing a new class like this: class myClass { public: myClass(); ^ } the caret drops back to column 1 after hitting return on the constructor declaration line (as indicated by the ^) The same is true for function declarations. Once the indentation was set up by pressing the tab key to the desired depth then all declarations get indented alright. * When adding a "protected" section later, then the "protected:" line is not unindented. Like so: class myClass { public: myClass(); ~myClass(); protected: ^ } Keep up the good work!
(In reply to comment #12) > Created an attachment (id=47827) [edit] Cool stuff! Are you working on a (default) formatter, too? I have collected some notes while reviewing and testing: - overall, the patch looks very good and clean. - the indenter seems to ignore the tab size of the Code Style page and still uses the tab size of the Text Editors page. - the indentation style is not respected when shifting left/right - the indenter should be used for template formatting, too - the option "use space for tabs" becomes obsolete - there should be options for different brace styles (e.g. indent braces), but that's an enhancement - we will need a bunch of unit tests for all those new features - contributed formatters may not respect all the code style settings; this should be mentioned on the page
(In reply to comment #15 and comment #14) I'm not working on the default formatter yet. I may look at it at some point, but it's not going to happen in the next two months. Im planning to spend some time to understand why the indexer correctly finds only 15-20% of classes in the code base I'm dealing with at my day job. I will try to address as many issues that you discovered as I can in the next week or two. Are there any existing CEditor unit tests? > - the indenter should be used for template formatting, too Java templates may optionally use code formatter, but not indenter. I prefer not to depart too far from JDT since CDT may need to borrow more JDT features later. > * When you are about it, why don't you make it so that for a new case statement in a switch the break is added automatically, similar to auto-closing braces, brackets and stuff? This is an enhancement request that goes beyond JDT. As you may have noticed, so far I've mostly collected low hanging fruit by adapting existing JDT code for use in CDT.
(In reply to comment #16) > (In reply to comment #15 and comment #14) > I will try to address as many issues that you discovered as I can in the next > week or two. Are there any existing CEditor unit tests? There is the CAutoIndentTest for testing auto-edit strategies without instantiating an editor (To test a realized editor, some more infrastructure is needed). I think most of the smart indenting can be regression-tested with this test class. Indenting and formatting has many possible cases (as comment 14 inidicates) which just need to have some automatic testing. At least the C/C++ specific cases should be covered initially. > > - the indenter should be used for template formatting, too > Java templates may optionally use code formatter, but not indenter. I prefer > not to depart too far from JDT since CDT may need to borrow more JDT features > later. Ok, but the java templates get indented even when the option to use the code formatter is disabled (see JavaFormatter). The template code must be indented according to the code style settings, there is no way around it.
Created attachment 48260 [details] Fixed few bugs and added a unit test - the indenter seems to ignore the tab size of the Code Style page and still uses the tab size of the Text Editors page. Fixed. - the indentation style is not respected when shifting left/right I'm not sure what you meant by this. Could you please give me the exact sequence of steps. - the indenter should be used for template formatting, too It does now. - the option "use space for tabs" becomes obsolete Removed. - case-statements get unindented alright when writing a switch, but the default: does not. It should be unindented like the cases. Fixed. - When writing a new class like this: class myClass { public: myClass(); ^ } Fixed. Also added a preference for indenting of public/protected/private. - When adding a "protected" section later, then the "protected:" line is not unindented. Like so: class myClass { public: myClass(); ~myClass(); protected: ^ } Fixed. - contributed formatters may not respect all the code style settings; this should be mentioned on the page Done. - we will need a bunch of unit tests for all those new features I've added a test for smart paste. The rest of functionality is dependent on CEditor.
(In reply to comment #18) > Created an attachment (id=48260) [edit] > Fixed few bugs and added a unit test Thank you for the quick fixes. > - the indentation style is not respected when shifting left/right > I'm not sure what you meant by this. Could you please give me the exact > sequence of steps. In mixed indent mode (tab width 8, indent width 4), shift left/right only adds/removes leading spaces, e.g. it does not turn a tab into 4 spaces when shifting left. But as JDT seems to handle it in the same way, it's an acceptable limitation. > - the indenter should be used for template formatting, too > It does now. Great. > - we will need a bunch of unit tests for all those new features > I've added a test for smart paste. The rest of functionality is dependent on > CEditor. Hm, I don't quite agree with the last sentence. All auto-indent features can be tested this way, but I will take on the task to add more tests and improve the test infrastructure for the editor. I consider the patch ready for commit and will initiate the IP legal process. I will probably contact you for input on certain items of the Contribution Questionaire (http://www.eclipse.org/legal/ContributionQuestionnairePart1-v1.0.php). Thanks!
(In reply to comment #19) > > I've added a test for smart paste. The rest of functionality is dependent on > > CEditor. > > Hm, I don't quite agree with the last sentence. Auto closing of brackets, strings and auto escaping when pasting to strings are not auto-indent features. It's not surprising that they are implemented in CEditor rather than in CAutoIndentStrategy. It would probably be cleaner to have all "auto-editing" in CAutoIndentStrategy given that it implements IAutoEditStrategy, but JDT does it differently, and I just followed JDT. > In mixed indent mode (tab width 8, indent width 4), shift left/right only > adds/removes leading spaces, e.g. it does not turn a tab into 4 spaces when > shifting left. But as JDT seems to handle it in the same way, it's an > acceptable limitation. I've filed bug 154574 against JDT. When somebody fixes it there, the fix can be ported to CDT.
Any plans at providing this as a plug-in that people could test with current installations, even if it isn't finished?
(In reply to comment #21) > Any plans at providing this as a plug-in that people could test with current > installations, even if it isn't finished? Definitely not. You need to build from source or wait until it is integrated into a nightly build.
Created attachment 49171 [details] Updated patch to apply cleanly to current HEAD I updated the patch because of recent overlapping changes to HEAD.
How is IP process going? Is there end in sight?
(In reply to comment #24) > How is IP process going? Is there end in sight? I am still waiting for PMC approval...
I think I, as PMC, am waiting for confirmation on who Sergey's employer is.
(In reply to comment #26) > I think I, as PMC, am waiting for confirmation on who Sergey's employer is. Seems to be a deadlock. Doug, according to http://www.eclipse.org/legal/ContributionQuestionnairePart1-v1.0.php and http://www.eclipse.org/legal/committerguidelines.php the PMC review is a technical one. The legal aspects are part of the EMO approval process. At least, this is how I understand it. Anyway, Sergey, could you make a statement about your employer and that you are authorized by your employer to make this contribution under the EPL? Thanks.
That's true, but I know how busy the Eclipse legal people are and if there are obvious issues, I'd rather catch them before sending things to them.
Do you have an(In reply to comment #27) > Anyway, Sergey, could you make a statement about your employer and that you are > authorized by your employer to make this contribution under the EPL? Do you have an example of such statement or an outline to fill in?
The contribution questionaire is at the link provided by Toni. The information we need from you is in the Contributors section near the bottom. And we need to ensure that the "Note:" has been satisfied although, as Toni mentions, Eclipse Legal will look into that anyway.
As Doug has pointed out, the Intellectual Property Group needs clarification that the contributor has authorization from their employer to contribute the code under EPL. We cannot move forward with the IP review process until we have that. Thanks.
The IP Group received the required authorizaton from Sergey Prigogin's employer for this code contribution under the EPL. The Contribution Questionnaire related to this code, is now "In Progress".
I have received approval for this contribution and committed the patch. It is available in builds > 20061002.
When will the formatter be avalable as a standard update? i.e. through the search for updates system?
(In reply to comment #35) > When will the formatter be avalable as a standard update? i.e. through the > search for updates system? As soon as CDT 4.0 gets released together with the Eclipse Europa simultaneous release, ie. end of June 2007.
(In reply to comment #36) > (In reply to comment #35) > > When will the formatter be avalable as a standard update? i.e. through the > > search for updates system? > > As soon as CDT 4.0 gets released together with the Eclipse Europa simultaneous > release, ie. end of June 2007. > Is there any way for the average CDT user to get hold of this feature before then? It's a pritty major addition to the functionality of the system and would be massively appreciated by most people who develop with CDT as there IDE.. Roja
If you want it now then the best solution is to use the cutting edge and deal with any injuries. In other words unless there is a decision to make a 3.x > 3.1, then you will have to get the code from version control, compile it and use that.
We are planning on releasing milestone builds for CDT 4.0 starting in December that will allow users to try new features out. These builds will definitely be use at your own risk but we will be managing them to ensure they are usable.
