Bug 221844 - ModificationResult.commit does not work correctly for .form files
ModificationResult.commit does not work correctly for .form files
Status: RESOLVED FIXED
Product: java
Classification: Unclassified
Component: Source
7.3
PC Windows 7
: P1 (vote)
: 7.3
Assigned To: Jan Lahoda
issues@java
: API, API_REVIEW_FAST
: 201922 222515 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-09 15:42 UTC by chinhodado
Modified: 2013-08-21 13:02 UTC (History)
6 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
IDE log (43.89 KB, text/plain)
2012-11-09 15:42 UTC, chinhodado
Details
Proposed API change. (21.76 KB, patch)
2012-11-16 18:21 UTC, Jan Lahoda
Details | Diff
Updated patch. (32.41 KB, patch)
2012-11-16 19:39 UTC, Jan Lahoda
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chinhodado 2012-11-09 15:42:23 UTC
Product Version = NetBeans IDE 7.3 Beta 2 (Build 201211062253)
Operating System = Windows 7 version 6.1 running on amd64
Java; VM; Vendor = 1.7.0_05
Runtime = Java HotSpot(TM) 64-Bit Server VM 23.1-b03

Renaming class is not working properly. for example, it still affects comments even when the option to affect comments is unchecked. Moreover, it messes up random parts of the code (can be easily seen using preview)
Comment 1 chinhodado 2012-11-09 15:42:30 UTC
Created attachment 127493 [details]
IDE log
Comment 2 Marian Mirilovic 2012-11-09 17:33:35 UTC
It is working for me in 
Product Version: NetBeans IDE Dev (Build 201211080001)
Java: 1.7.0_10-ea; Java HotSpot(TM) 64-Bit Server VM 23.6-b04

... could you please share the class/project with us ? Thanks in advance.
Comment 3 chinhodado 2012-11-11 02:53:05 UTC
The project can be found here:

https://github.com/chinhodado/tictactoe-ocsf

Here is a screenshot:

http://s12.postimage.org/89a1f9ut9/screenshot_11.jpg

I preview the change of class name from GameClientGUI to GameClientGUIABC. Rename in comment option is not checked. Note that:

- comments are still affected, or should I say corrupted
- some random comments are added
- keywords try and break are also corrupted
Comment 4 Ralph Ruijs 2012-11-12 12:46:56 UTC
It seems that after http://hg.netbeans.org/jet-main/rev/b2e7797ae98e the positions from the ModificationResult are no longer the same as in the file.

Problem is similar to #189203.
Comment 5 Tomas Pavek 2012-11-12 18:41:47 UTC
I've tried the given example and can see the problem only in the preview. The refactoring itself worked well for me. That would mean that only the preview is not correct. Is there a case that really breaks the really performed refactoring?

Just for the record, before the mentioned change b2e7797ae98e the positions were not guaranteed either. They fit when a form was just opened - thanks to the extra space chars. After regenerating the guarded sections and saving there were no spaces in the opened document that would match the guarded comments in the file anymore.

I felt it would be cleaner to have a consistent behavior that can be guaranteed (which is no extra spaces). I was not aware of that this breaks refactoring preview. If needs be, we can have the spaces back.
Comment 6 Jan Lahoda 2012-11-12 19:03:45 UTC
(In reply to comment #5)
> I've tried the given example and can see the problem only in the preview. The
> refactoring itself worked well for me. That would mean that only the preview is
> not correct. Is there a case that really breaks the really performed
> refactoring?

The easiest way is to close the form while the Preview is open and then "Do Refactor". There are more cases where it break (do the refactoring with the form closed).

> 
> Just for the record, before the mentioned change b2e7797ae98e the positions
> were not guaranteed either. They fit when a form was just opened - thanks to
> the extra space chars. After regenerating the guarded sections and saving there
> were no spaces in the opened document that would match the guarded comments in
> the file anymore.

True, I always wondered how comes it works OK. Now I know it did not work. The main difference is that before the one had to use a little bit more complicated steps to reproduce (you would actually need to close the form after running the refactoring which needed to run after the form was modified), but now it is much more common.

> 
> I felt it would be cleaner to have a consistent behavior that can be guaranteed
> (which is no extra spaces). I was not aware of that this breaks refactoring
> preview. If needs be, we can have the spaces back.

Hm, I guess the only realistic remaining option is to open the documents during the ModificationResult commit for forms (which is probably the easier part), and to pass the source through the GuardedReader when creating the snapshot for a non-open file (which is probably the hard part).
Comment 7 Jan Lahoda 2012-11-16 18:21:18 UTC
Created attachment 127955 [details]
Proposed API change.

This patch ensures that when the content of the guarded section is set (through editor.guards API only), its content is passed through the corresponding writer&reader, allowing them to re-create any characters as if the data would be written to disk and read back. This allows (with help from the given readers&writers) to ensure that offsets on disk and in the editor are the same.
Comment 8 Jan Lahoda 2012-11-16 18:21:42 UTC
Please review.
Comment 9 Jan Lahoda 2012-11-16 19:39:17 UTC
Created attachment 127961 [details]
Updated patch.
Comment 10 Tomas Pavek 2012-11-19 18:53:37 UTC
It's not easy reading, but I think it should work. It's good the actual change in API is really minimal.

One thing I noticed in GuardedSectionImpl.setText: looks like the sections are read twice:
    Result result = guards.gr.readSections(data);
    List<GuardedSection> guardedSections = guards.gr.readSections(data).getGuardedSections();
Comment 11 Ralph Ruijs 2012-11-21 09:07:51 UTC
*** Bug 222515 has been marked as a duplicate of this bug. ***
Comment 12 Jan Lahoda 2012-11-25 21:48:40 UTC
(In reply to comment #10)
> It's not easy reading, but I think it should work. It's good the actual change
> in API is really minimal.
> 
> One thing I noticed in GuardedSectionImpl.setText: looks like the sections are
> read twice:
>     Result result = guards.gr.readSections(data);
>     List<GuardedSection> guardedSections =
> guards.gr.readSections(data).getGuardedSections();

Good catch, thank you very much!

Applied:
http://hg.netbeans.org/jet-main/rev/caa9e39ad74c
Comment 13 Quality Engineering 2012-11-26 03:14:05 UTC
Integrated into 'main-golden', will be available in build *201211260002* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/caa9e39ad74c
User: Jan Lahoda <jlahoda@netbeans.org>
Log: #221844: when a guarded section's part is set, pass the new content through the guarded writer&reader, to achieve results as similar to saving&loading a possible, to allow keeping the same positions between file on disk and Document content
Comment 14 Tomas Pavek 2012-11-26 18:37:15 UTC
Made an additional fix that removes previous hack in form editor trying to keep the guarded comments its own way.
Plus fixed renaming of sections in the mode that keeps the guarded comments. In this case when the renamed section is written, it still has the guard comment with the previous name in its text (in the document) which needs to be removed.
http://hg.netbeans.org/jet-main/rev/c21b08be1122
Comment 15 Quality Engineering 2012-11-27 02:56:02 UTC
Integrated into 'main-golden', will be available in build *201211270002* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/c21b08be1122
User: Tomas Pavek <tpavek@netbeans.org>
Log: addition to fix of #221844: removing temporary hack in form that is no longer needed, fixing writing of renamed sections when guarded comments are kept
Comment 16 Miloslav Metelka 2013-08-21 13:02:03 UTC
*** Bug 201922 has been marked as a duplicate of this bug. ***


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo