Community
Participate
Working Groups
Version: 3.0.0 Build id: 200406192000 The code formatter fails silently if a file contains Java 1.5-specific features. While it is fully understandable that the formatter will not be updated until all the 1.5 features have been implemented, some kind of feedback would be much appreciated.
true
Moving to J Core to clarify and maybe extend the API. The CodeFormatter.format API says: * Format <code>source</code>, * and returns a text edit that correspond to the difference between the given string and the formatted string. * It returns null if the given string cannot be formatted. What get returned if there is no difference? An empty edit? It cannot be null since null means: cannot be formatted. To fix this bug we should throw an exception which explains why it failed.
Answering parts of comment 2: the case where nothing is changed is handled by an empty MultiTextEdit.
Throwing an exception would break the existing API and therefore the existing clients.
So a null return from CodeFormatter.format is *always* a serious failure? Or are there situations in which a string cannot be formatted, but there's no reason to tell the user either? If it's always a failure, can the user-triggered formatter be made to produce a failure popup message in 3.0, and the compatibility-breaking comprehensive feedback system be left for a more serious API change?
Indeed null is always a bad failure. It either means that the input string is formatted using a wrong kind or the input string cannot be formatted (missing tokens, incomplete or invalid source). In your case, the formatter fails because the 1.5 code is not handled, then the formatter doesn't find the expected tokens while formatting and it stops. It is up to the client to define what should be the failing strategy. Move to JDT/Text to add feedback for the use when the formatter fails. On my side I will add the 1.5 constructs in the code formatter.
I still think format should throw an exception which explains why it fails, for example would you like if when you save a file with the editor it simply says: Did not work. With any hint to fix it? I bet we'd have a bug report from you in no time ;-) Also, API is not an excuse here. You can offer a second method. We have tons of those in text land. If we wouldn't fix things simply because of API then we were still at Eclipse R2.0 level. Moving back for second consideration.
Jim, What do you think? I disagree when Dani says that we should trow an exception. Returning null or throwing an exception doesn't change anything. The formatter failed. Period. This is always a internal failure. The user is supposed to know what kind should be used to format the code. It is not up to the formatter to report such errors. Right now we have two cases: - A text edit is returned. This means the formatting worked and the text edit should be applied on the document. - null is returned. The formatter failed. This could be due to a wrong kind used to format the code or the formatter has a internal error. So I don't see why an exception would help. I propose that API with an exceception a while ago, and if I remember well, Martin didn't like it. So let's wait that Martin is back and we will talk with him.
>I disagree when Dani says that we should trow an exception. Returning null or >throwing an exception doesn't change anything. Why did you propose it then in the first place? ;-)
I preferred the exception case, but it has not been chosen. This is why I don't want it to be change now. We already have one deprecated api for code formatting. I don't want to end up with another one.
This much is clear: the formatter returns null if it cannot format the document given it, as distinct from reformatting the document resulting in no net change. This is all that a client needs to report the fact that a problem has occurred when an attempt to format fail. If JDT UI did this, it would address Charlie's complaint that the formatter is failing silently. Without further info, the error needs to be worded vaguely, like "Unable to format Java code (check syntax)". But are there there cases where this wording would not be sufficient to alert the user to a problem?
I agree with Jim. Because the formatter can fail for different reasons, it is impossible to give a good explanation to the user. The best guess is a syntax error in the code. If the formatter fails (the UI could check for the null returned value), the best solution is to open a bug report against JDT/Core and we will investigate it.
Olivier, why did you first suggest to throw an exception instead of returning null if no other information except "it failed" can be provided by J Core? Or is this not fully true i.e. you can differ from different "null" cases? I assumed that J Core could report more details about why it failed. If this is not the case then null is fine with me and we will simply report this in the UI.
I won't argue over and over again about throwing an exception or not. The API doesn't trow an exception. My point in the past was that an exception is closer to a failure case than a null case. The UI now simply needs to check for the null case. We had a feedback in the .log at some point in the past using the same API. So it is doable. If the UI reports the problem in the .log file, it would be nice to get all the information provided to the code formatter in the error trace. If the code is saved, the compiler can report syntax error problems and potential problems that could prevent the formatter to work. I don't think adding a new API in this case would help. To fix this issue from the JDT/Core side, we need to add the handling of 1.5 constructs in the code formatter. Reporting feedback to the user is a UI task. The UI knows that the code formatter failed, because null is returned.
Moving to UI for further action.
Deferred.
Resetting priority to P3. Will be reassessed for the next release.
*** Bug 52745 has been marked as a duplicate of this bug. ***
Has to be shifted to 3.3.
The code location that gets the 'null' back does not have access to the editor or any other element that would be able to provide the status line or a shell to show an error dialog. It would involve several API changes to pass this back to the editor. This is too much work (and API changes) for now for this minor problem.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
