Community
Participate
Working Groups
We're currently making calls to internal jdt.ui code in order to access the project specific tab settings - CodeFormatterUtil.getTabWidth(IJavaProject). Is there an alternate way to get this setting? If not, it needs to be refactored into public API. This is blocking our effort to move away from jdt internal usage.
One could days that you should just use project.getOption(DefaultCodeFormatterConstants.FORMATTER_TAB_SIZE) but I agree, things got quite complicated with the addition of the mixed tab/char mode. One needs to understand 'indentWidth', 'tabWidth' and how you get these depending on the selected tab char. There's much more functionality regarding indentation (evaluate, change indentation of source, create indented source) that I think this should be added to the code formatter code. I suggest to make the class 'Indents.java' public. Indents.java is already in jdt.core, in 'org.eclipse.jdt.internal.core.dom.rewrite'. Me and Tom just went over the API, improved names, completed the Javadoc, so it should ready for being API
Moving to jdt.core. Can you consider making Indents API in the formatter package? I would volonteer to maintain it.
Jim, any thought on that move?
A couple of nits: - In which API package would this class fit best? o.e.jdt.core.utils might be a fallback if there is no better place. - The new API class should be tagged @since 3.2 and marked final for extra measure (it already has a private constructor).
My suggestion is to put it in the same package as the formatter. Changing indents is related to formatting, and it is also the formatter which introduces the notion of an indentation level in 'format(int kind, String source, int offset, int length, int indentationLevel, String lineSeparator)' I just realize that we might also use indentationLevel instead of indentationUnits in Indents to be consistent here. It could also be discussed to put the API 'createIndentString' on CodeFormatter itself, as code formatters can have their own strategies how an indendatation in the mixed tab/char case is created. -> CodeFormatter.createIndentationString(int indentationLevel)
Jim, Olivier, Philippe? Any objections, opinions for this as new API? As mentioned in comment 5, I would suggest to add CodeFormatter.createIndentationString(int indentationLevel) to the code formatter itself.
Philippe, do we want to add this for 3.2?
The problem is that I cannot add an abstract method in CodeFormatter since this can be subclassed by client. It is not specified explicitely that it cannot be subclassed, so by default it is possible to subclass it.
You could add it as a normal, non abstract method that either a.) uses a default algrorithm to calculate the indentation. b.) calculates the indentation using format -> format(K_EXPRESSION, "x", 0, 1, indent, '\n'), calculate the result and extract the indendation string from the formatted result. As b.) is rather inefficient (get edits, apply edits...) the real CodeFormater implementation should override this method with a more performant one.
I will add the following concrete method in org.eclipse.jdt.core.formatter.CodeFormatter. /** * Answers the string that corresponds to the indentation to the given indentation level or null if the indentation cannot * be computed. * <p>This method needs to be overriden in a subclass.</p> * * <p>The default implementation returns null.</p> * * @param indentationLevel the given indentation level * @return the string corresponding to the right indentation level */ public String createIndentationString(int indentationLevel) { return null; } Since the default implementation provided by us will always returns a non-null result, this is fine. Let me know what you think.
I would prefer a empty string. Otherwise I -as client- always have to make a null check as the spec mentions that there could be null (I'm programming against the abstract class and have to assume that I might (in the future) also see a formatter different than the default formatter)
That's fair. I'll do some testing in different modes to be sure I get something consistent. Would you have specific test cases that you want to be covered in regression tests?
I would expect that the results of your implementation and our (Indents.createIndentString) will result in the same (there's not that much you can do differently, I think) In jdt.ui we have the same method in CodeFormatterUtil.createIndentString When I redirect CodeFormatterUtil.createIndentString to your new method I will see if any tests are concerned. But I'm not expecting any problems, we mostly use trivial indentations in our tests (all spaces, or tab, not mixed)
Then the easiest way yo do this is to move your implementation in Indents.createIndentString into the code formatter. I checked it and I believe it is covering all cases.
Also I would add that an IllegalArgumentException is thrown if the given indentation level is lower than 0.
Fixed and released in HEAD. Regression test added in org.eclipse.jdt.core.tests.formatter.FormatterRegressionTests.test611. Martin, I will patch CodeFormatterUtil.createIndentString and ensure that all your tests are passing.
Can we also move 'Indents' to the codeformatter package now? (that was the original request) createIndentationString can now be removed from 'Indents'.
Then why did we add an API on code formatter? This could simply remain on Indents.
Indents has been moved as API in org.eclipse.jdt.core.formatter.IndentManipulation. I added @exception and changed a bit the javadoc. Let me know if this is fine. Martin agreed to add specific regression tests and to maintain this new API.
Verified for 3.2 M5 using build I20060214-0010. The APIs are there. No test fails. Comment #18 still raises an interesting question.
