Bug 156357 - StatusDisplayer API enhancement review
StatusDisplayer API enhancement review
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Window System
6.x
All All
: P2 (vote)
: 6.x
Assigned To: Stanislav Aubrecht
issues@platform
: API_REVIEW_FAST
Depends on:
Blocks: 123344 153283
  Show dependency treegraph
 
Reported: 2009-01-06 14:08 UTC by Stanislav Aubrecht
Modified: 2009-02-19 22:53 UTC (History)
4 users (show)

See Also:
Issue Type: TASK
:


Attachments
new patch (5.42 KB, patch)
2009-01-13 14:08 UTC, Stanislav Aubrecht
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stanislav Aubrecht 2009-01-06 14:08:25 UTC
Because of merging of editor's status line with main window's status line it is necessary to define the 'importance' of
messages being displayed in the main status line. Messages with higher importance will replace messages with lower
importance.
These important messages will stay permanently visible until explicitly cleared or replaced (as opposed to current
implementation when all status line messages are removed after some time).

The API change introduces new method public abstract void setStatusText(String text, int importance) to
org.openide.awt.StatusDisplayer
Comment 1 Stanislav Aubrecht 2009-01-06 14:10:08 UTC
proposed api changes:

# This patch file was generated by NetBeans IDE
# Following Index: paths are relative to: D:\hg\core-main\openide.awt
# This patch can be applied using context Tools: Patch action on respective folder.
# It uses platform neutral UTF-8 encoding and \n newlines.
# Above lines and this line are ignored by the patching process.
Index: apichanges.xml
--- apichanges.xml Base (BASE)
+++ apichanges.xml Locally Modified (Based On LOCAL)
@@ -47,6 +47,24 @@
 <apidef name="awt">AWT API</apidef>
 </apidefs>
 <changes>
+    <change id="StatusTextImportance">
+        <api name="awt"/>
+        <summary>Allow messages to show in the main status line permanently.</summary>
+        <version major="7" minor="5"/>
+        <date day="6" month="1" year="2009"/>
+        <author login="saubrecht"/>
+        <compatibility addition="yes" binary="compatible" semantic="compatible" deprecation="no" deletion="no"
modification="no"/>
+        <description>
+        Because of merging of editor's status line with main window's status line
+        it is necessary to define the 'importance' of messages being displayed in the
+        main status line. Messages with higher importance will replace messages
+        with lower importance. These important messages will stay permanently visible
+        until explicitly cleared or replaced (as opposed to current implementation
+        when all status line messages are removed after some time).
+        </description>
+        <class package="org.openide.awt" name="StatusDisplayer"/>
+        <issue number="123344"/>
+    </change>
     <change id="Actions.MenuText">
         <api name="awt"/>
         <summary>Actions can have "menuText" and "popupText" properties</summary>
Index: nbproject/project.properties
--- nbproject/project.properties Base (BASE)
+++ nbproject/project.properties Locally Modified (Based On LOCAL)
@@ -46,4 +46,4 @@
 #javadoc.apichanges=${basedir}/api/apichanges.xml
 javadoc.apichanges=${basedir}/apichanges.xml
 
-spec.version.base=7.4.0
+spec.version.base=7.5.0
Index: src/org/openide/awt/StatusDisplayer.java
--- src/org/openide/awt/StatusDisplayer.java Base (BASE)
+++ src/org/openide/awt/StatusDisplayer.java Locally Modified (Based On LOCAL)
@@ -89,11 +89,28 @@
      *  <p class="nonnormative">Default implementation of status line in NetBeans
      * displays the text in status line and clears it after a while. 
      * Also there is no guarantee how long the text will be displayed as 
-     * it can be replaced with new call to this method at any time.
+     * it can be replaced with new call to this method at any time.</p>
+     * <p>Note: The text may not show in the status line at all if some
+     * other text with higher importance is currently showing in the status line.</p>
      * @param text the text to be shown
+     * @see #setStatusText(String,int)
      */
     public abstract void setStatusText(String text);
 
+    /**
+     * <p>Show text in the status line. Positive and non-zero <code>importance</code> argument
+     * indicates that the text should stay in the status line until it is replaced
+     * with new text by calling <code>setStatusText(String,int)</code> again with
+     * the same or higher importance value.</p>
+     * <p>Negative or zero <code>importance</code> argument is equivalent of calling
+     * <code>setStatusText(String)</code> as such messages will disappear after a while.</p>
+     * @param text The text to be shown until some other text with the same or higher
+     * importance is passed into the status line.
+     * @param importance 'Importance' of the message to be displayed, the higher number
+     * the higher importance.
+     */
+    public abstract void setStatusText(String text, int importance);
+
     /** Add a listener for when the text changes.
      * @param l a listener
      */
@@ -130,6 +147,10 @@
             cs.fireChange();
         }
 
+        public void setStatusText(String text, int importance) {
+            setStatusText(text);
+        }
+
         public void addChangeListener(ChangeListener l) {
             cs.addChangeListener(l);
         }
Comment 2 Miloslav Metelka 2009-01-06 15:04:44 UTC
I agree with the proposed change.
Maybe we could mention some hint for the positive values e.g. 100 for a warning and 200 for an error etc. just to have a
hint for clients.
Comment 3 Vitezslav Stejskal 2009-01-07 11:47:30 UTC
VS1:
> +     * <p>Note: The text may not show in the status line at all if some
> +     * other text with higher importance is currently showing in the status line.</p>

I'm little bit concerned about this. Since high importance messages have to be cleared off explicitly failing to do so
may  end up with status bar showing _only_ the high importance messages. Maybe this will not be a real life problem and
it certainly seems more like a problem of the implementation behind the API. So the change is probably ok.

Could you please elaborate more on why this is needed and how this is going to be used (I suppose by the editor)? Thanks
Comment 4 Petr Hejl 2009-01-07 12:11:07 UTC
PH01: Do we really need whole int range for importance? I think it would be better to define limited set of "importance
levels" - similar way as it is done for any logging framework. Otherwise the whole importance parameter usage could
reduce to 0 (I don't care about the real importance) and Integer.MAX_VALUE (I want to display this).

Can you provide more detail use cases? Especially importance-visibility related ones.
Comment 5 Stanislav Aubrecht 2009-01-07 12:37:36 UTC
VS01: 'important' message types are described in the ui spec:
http://ui.netbeans.org/docs/ui/CommonStatusbar/CommonStatusLineSpecification.html
all such messages are related to some user activity in the IDE so as soon as the activity stops the message will be
cleared - same way is it works in editor status bar now.

PH01: logging level are just some well-known integer values so we can do the same for message importance:
int ANNOTATION = 500;
int FIND_OR_REPLACE = 400;
int DIFF_DETAIL = 300;
the status line is a platform component so it's not very well possible to define only a small set of allowed values as
it would restrict platform application development.

some use cases are in the ui spec
Comment 6 Miloslav Metelka 2009-01-07 13:36:45 UTC
> all such messages are related to some user activity in the IDE so as soon as the activity stops the message will be
> cleared - same way is it works in editor status bar now.

Not true - e.g. for searching the message "search-str found at line:col" gets cleared not upon finishing of search
action but with the next action being invoked e.g. a cursor movement or typing.
IMHO setStatusText(null, importance) could notify that the particular importance-level is no longer occupied so the
subsequent setStatusText(text, lower-importance) would not get ignored. The fragile thing is to ensure that
setStatusText(null, importance) would always get called.


Comment 7 Stanislav Aubrecht 2009-01-07 13:49:27 UTC
>> all such messages are related to some user activity in the IDE so as soon as the activity stops the message will be
>> cleared - same way is it works in editor status bar now.

>Not true - e.g. for searching the message "search-str found at line:col" gets cleared not upon finishing of search
action but with the next action being invoked e.g. a cursor movement or typing.
that's my point - there already is some piece of code that clears editor status line text so it'll be simply reused to
clear common status line as well.

if there's a safer pattern for status line text handling, i'm open to suggestions...
Comment 8 Jaroslav Tulach 2009-01-07 15:13:22 UTC
Y01 You cannot add new abstract method into the class. Add default body that delegates to the previously existing 
method.

Y02 I am concerned by the need to clear message. I am almost sure that someone will forget, as people forget to close 
streams. Maybe somehow connect this feature to garbage collector? Like: 

public Message setStatusText(String txt, int priority)
public static final class Message {
  public void clear(int timeInMillis);
  protected void finalize() {
    clear(0);
  }
}
Comment 9 Jesse Glick 2009-01-13 00:43:41 UTC
For future patches, please use 'hg diff -g' format if possible, and attach them rather than pasting inline.


[JG01] apichanges.xml points to a different issue.


[JG02] "Positive and non-zero" is redundant.


[JG03] Having <=0 behave as a different method is confusing. Rather make this condition illegal (IAE).
Comment 10 Stanislav Aubrecht 2009-01-13 14:08:00 UTC
Created attachment 75757 [details]
new patch
Comment 11 Stanislav Aubrecht 2009-01-13 14:08:48 UTC
i've attached a new patch which should address all reviewers comments
Comment 12 Jesse Glick 2009-01-13 14:20:00 UTC
[JG04] Message should have not be a public nonfinal class with a public constructor, I guess. Since I assume that a
subclass of StatusDisplayer is intended to override setStatusText(String,int), and would likely need to implement
Message.clear, it should probably be an interface. The default impl present in the API module also needs to function
somehow so that code run in unit tests will not be broken.


[JG05] Typo: "excplicitly"


[JG06] Javadoc should (a) not duplicate text "The text will be removed..."; (b) clarify that setStatusText(String) can
be considered of zero importance (right?); (c) clarify that sST(String,int) with a lower importance, or sST(String),
will succeed if a higher-priority message has already been cleared; (d) clarify whether clearing a message fires a
change event.
Comment 13 Stanislav Aubrecht 2009-01-26 08:57:41 UTC
if there are no more comments i'll integrate this enhancement sometime this week

i'll also change StatusDisplayer.Message to an interface and update javadoc as requested
Comment 14 Jaroslav Tulach 2009-01-26 12:14:46 UTC
Y11 Interface!? What if you find a need for method like "blink()" or "changeImportance", etc. What will you do? Can 
you describe your evolution plan in this case?
Comment 15 Stanislav Aubrecht 2009-01-26 12:26:46 UTC
Y11: introduce a new interface that inherits from the old one?
Comment 16 Stanislav Aubrecht 2009-01-27 14:58:10 UTC
integrated in 854050c00382
Comment 17 Quality Engineering 2009-01-28 09:52:23 UTC
Integrated into 'main-golden', will be available in build *200901280348* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/854050c00382
User: S. Aubrecht <saubrecht@netbeans.org>
Log: #156357 - added 'importance' support for status line messages


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