This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Summary: | Need version of SHOW_REUSE which doesn't grab the focus | ||
---|---|---|---|
Product: | platform | Reporter: | ivan <ivan> |
Component: | Text | Assignee: | mslama <mslama> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | anebuzelsky, apireviews |
Priority: | P2 | Keywords: | API_REVIEW_FAST |
Version: | 6.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | DEFECT | Exception Reporter: | |
Attachments: |
Patch for this issue
Update deprecated tags for old Line constants Use non abstract Line.show(ShowOpenType, ShowVisibilityType, int) Give more specific warning message |
Description
ivan
2008-06-24 20:20:02 UTC
This suggestion seems reasonable to me. Current constants in Line class would stay there for backward compatibility and new bit constants would be added. Main reason is to split behavior into categories as described above so that we need not provide constanst for all possible combinations. I need opinion from another people before I start to implement this change. [JG01] Use an enumeration rather than integer flags. You can use EnumSet to compose them. Yes EnumSet is fine. I checked SHOW_REUSE and SHOW_REUSE_NEW and these have different behavior so they must have their own new flags. It means it is not possible to use suggested combinations. In addition /** Replacement for constants SHOW_TRY_SHOW, SHOW_SHOW, * SHOW_GOTO, SHOW_TOFRONT, SHOW_REUSE, SHOW_REUSE_NEW. It is to provide full control * over show method behavior without need to add new constant for missing flag combination. * SHOW_TYPE_OPEN if set new editor is opened when it is not yet opened * SHOW_TYPE_FOCUS if set editor is focused/activated it implies SHOW_TYPE_FRONT so using SHOW_TYPE_FOCUS and * SHOW_TYPE_FRONT is redundant * SHOW_TYPE_FRONT if set editor is fronted/selected * SHOW_TYPE_REUSE if set replaces editor marked for reuse (ie. editor opened with last SHOW_TYPE_REUSE * or SHOW_TYPE_REUSE_NEW) and if editor is not yet opened marks it for reuse * SHOW_TYPE_REUSE_NEW if set ignores editor marked for reuse and resets reference to editor marked for reuse * and if editor is not yet opened marks it for reuse * * Note: SHOW_TYPE_REUSE and SHOW_TYPE_REUSE_NEW flags cannot be used together. * * Note: Any modification of editor marked for reuse resets reuse flag. There is one global static reference * so only one or none editor can be marked for reuse. * * Replacement for old constants: * SHOW_TRY_SHOW {} * SHOW_SHOW {SHOW_TYPE_OPEN} * SHOW_GOTO {SHOW_TYPE_OPEN, SHOW_TYPE_FOCUS} * SHOW_TOFRONT {SHOW_TYPE_OPEN, SHOW_TYPE_FRONT} * SHOW_REUSE {SHOW_TYPE_REUSE, SHOW_TYPE_FOCUS} * SHOW_REUSE_NEW {SHOW_REUSE_NEW, SHOW_TYPE_FOCUS} */ public enum ShowTypes { SHOW_TYPE_OPEN, SHOW_TYPE_FOCUS, SHOW_TYPE_FRONT, SHOW_TYPE_REUSE, SHOW_TYPE_REUSE_NEW }; Flags SHOW_TYPE_REUSE and SHOW_TYPE_REUSE_NEW imply SHOW_TYPE_OPEN but as it makes no sense to use these 2 without SHOW_TYPE_OPEN I do not split 'open' and 'reuse'. It means it makes no sense to combine {SHOW_TYPE_OPEN, SHOW_TYPE_REUSE} or {SHOW_TYPE_OPEN, SHOW_TYPE_REUSE_NEW}. Also flags {SHOW_TYPE_REUSE, SHOW_TYPE_REUSE_NEW} cannot be combined. Possible combinations are {} {SHOW_TYPE_OPEN} {SHOW_TYPE_OPEN, SHOW_TYPE_FOCUS} {SHOW_TYPE_OPEN, SHOW_TYPE_FRONT} {SHOW_TYPE_REUSE} {SHOW_TYPE_REUSE, SHOW_TYPE_FOCUS} {SHOW_TYPE_REUSE, SHOW_TYPE_FRONT} {SHOW_TYPE_REUSE_NEW} {SHOW_TYPE_REUSE_NEW, SHOW_TYPE_FOCUS} {SHOW_TYPE_REUSE_NEW, SHOW_TYPE_FRONT} So this is not pure bit mask with all possible values. So there is question if this is right way. In any way current impl does not allow to separate 'open' from 'reuse'. Would it make any real sense to separate it? I missed following combinations: {SHOW_TYPE_FOCUS} {SHOW_TYPE_FRONT} As I look at it now open and visibility types are well separated. So I could use 2 params instead of one. public enum ShowOpenTypes { SHOW_OPEN_TYPE_NONE, SHOW_OPEN_TYPE_OPEN, SHOW_OPEN_TYPE_REUSE, SHOW_OPEN_TYPE_REUSE_NEW }; public enum ShowVisibilityTypes { SHOW_VISIBILITY_TYPE_NONE, SHOW_VISIBILITY_TYPE_FRONT, SHOW_VISIBILITY_TYPE_FOCUS }; These 2 values can be combined in any way and cover all old cases. [IS01] Just to make sure ... public enum ShowVisibilityTypes { SHOW_VISIBILITY_TYPE_NONE, SHOW_VISIBILITY_TYPE_FRONT, SHOW_VISIBILITY_TYPE_FOCUS // implies SHOW_VISIBILITY_TYPE_FRONT ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ }; [IS02] Can all the combinations be implemented? If not, What happens to invalid combinations? is01: Yes is02: Yes all couples are possible. First 'open' part is done (ie. one action from ShowOpenTypes). And then visibility action is performed. These 2 actions are well separated. Created attachment 64001 [details]
Patch for this issue
Jarda agreed to go for fasttrack. Please someone read new javadoc if it is clear enough. Especially REUSE and REUSE_NEW have more complex behavior. I added note about one global reference to reusable editor to make things clearer. IMO if some developer is new to this API (like me) it is not obvious. [JG02] Can you use column == -1 (no particular column) for the new method? The Javadoc does not say. [JG03] @deprecated messages should specify which combination to use. Please also consider updating existing modules to use the preferred API style. Ad JG02: Does it make any sense to use -1 for column? What should it do? It means undefined (random or first column) column will be selected. As editor cursor must be somewhere. It is as show without column translated to 0 column. Ad JG03: Ok I will add new combination to @deprecated messages. There is translation table in javadoc for ShowOpenType. If I add this to old constants shall I remove it from this place? I will try find usages on full IDE to find out Line.show usages and see. I will probably file issues and rewrite usage if module owner(s) will agree. I will get to it next week. I plan to commit this during weekend. To JG02 - that's fine but it is not specified in the Javadoc. javadoc of show without column parameter says: "Shows the line (at the first column)" It is enough IMO. I moved replacement constants to @deprecated tag for every old constant. I will commit this tomorrow evening. Created attachment 64352 [details]
Update deprecated tags for old Line constants
Correction for one link here: /** Focuses or opens given editor, marking it as reusable editor if it * was not opened before. Similar to {@link #SHOW_REUSE} but ignores * currently reusable editor. * @see #show(int) <code>show</code> * @since org.openide.text 6.14 * @deprecated Deprecated since 6.21. Use {@link ShowOpenType#SHOW_OPEN_TYPE_REUSE_NEW} * and {@link ShowVisibilityType#SHOW_VISIBILITY_TYPE_FOCUS} instead. */ public final static int SHOW_REUSE_NEW = 5; To JG02 - no, it's not enough, because "-1" is not mentioned anywhere, yet that is what you in fact need to pass to the new method if you wish to not change the column. That, or add a new method override that takes your new constants but not a column number. Ahh I see now: if (column >= 0) { in code. It is not mentioned anywhere. I will add: Pass -1 if you do not want to change selected column. It applies to old show method too so I add the same to it. Fixed as 9d3449089017. -1 for column added to javadoc of method show. Module version increased in module manifest - it was missing i diff. Sigtest found incompatible change: I did not know it is not possible to add abstract method into API. So I had to provide simple impl of Line.show(ShowOpenType openType, ShowVisibilityType visibilityType, int column). It fallbacks to show(int,int) and logs warning when not supported params are passed into method. Proper impl is provided by org.openide.text.EditorSupportLineSet.SupportLine. Diff attached. Sigtest now passes. I put it to main as it breaks continuous build. Created attachment 64424 [details]
Use non abstract Line.show(ShowOpenType, ShowVisibilityType, int)
Created attachment 64425 [details]
Give more specific warning message
Fixed in main 5f55df5841aa Integrated into 'main-golden', available in NB_Trunk_Production #324 build Changeset: http://hg.netbeans.org/main/rev/5f55df5841aa User: Marek Slama <mslama@netbeans.org> Log: #138146: Do not add abstract method to API. I suppose it is too late to comment that [JG04] SHOW_VISIBILITY_TYPE_ and SHOW_OPEN_TYPE_ prefixes on enum constant names are redundant and annoying. Calling .show(Line.ShowOpenType.SHOW_OPEN_TYPE_OPEN, Line.ShowVisibilityType.SHOW_VISIBILITY_TYPE_FOCUS) is just verbose and no clearer than .show(Line.ShowOpenType.OPEN, Line.ShowVisibilityType.FOCUS) It is after feature freeze. Tonda? Cosmetic change? As I did not get into rewriting client code yet this rename would limited to openide/text only. ...and now o.apache.tools.ant.module also. > Tonda? Cosmetic change?
Better do it now before it gets widely used. So, do it.
Fixed as 8c2b7fcc16bd and f2f8bd75346d in core-main. I'm sorry, but after all this good effort the original problem I had is still not solved. When I use line.show(Line.ShowOpenType.REUSE, Line.ShowVisibilityType.FRONT); focus will get grabbed away in the case where a tab is actually reused. Alter the testcase project provided in 132671 with this: private void showInEditor(Line line, boolean reuse) { if (reuse) // OLD line.show(Line.SHOW_REUSE); line.show(Line.ShowOpenType.REUSE, Line.ShowVisibilityType.FRONT); else // OLD line.show(Line.SHOW_SHOW); line.show(Line.ShowOpenType.OPEN, Line.ShowVisibilityType.FRONT); } then issue commands edit -r filea edit -r fileb edit -r filec will reuse and grab focus away Issue #132671 contains testing module. I tried and cannot reproduce your problem. I modified source in test module as described: private void showInEditor(Line line, boolean reuse) { if (reuse) { //line.show(Line.SHOW_REUSE); line.show(Line.ShowOpenType.REUSE, Line.ShowVisibilityType.FRONT); } else { //line.show(Line.SHOW_SHOW); line.show(Line.ShowOpenType.OPEN, Line.ShowVisibilityType.FRONT); } } then use build and run. Created 3 source files a,b,c and used commands as mentioned above cd (to get to home dir) pwd (to check current work dir) edit -r a edit -r b edit -r c edit -r a Focus stays in shell window not moved to editor. Anyway please file separate issue to avoid long issue and give steps to reproduce it there. I am on Linux, latest trunk dev build, JDK 6u10: Product Version = NetBeans IDE Dev (Build 080910) Operating System = Linux version 2.6.24-21-generic running on i386 Java; VM; Vendor = 1.6.0_10; Java HotSpot(TM) Client VM 11.0-b15; Sun Microsystems Inc. Runtime = Java(TM) SE Runtime Environment 1.6.0_10-b30 I have no idea what is the difference causing this. I tried even clean and build. |