[Jmol-developers] [ jmol-Feature Requests-1521473 ] Better STDOUT/STDERR support

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 454-5900

Feature Requests item #1521473, was opened at 2006-07-12 23:26
Message generated for change (Comment added) made by nicove
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=379136&aid=1521473&group_id=23629

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Priority: 5
Submitted By: Egon Willighagen (egonw)
Assigned to: Nobody/Anonymous (nobody)
Summary: Better STDOUT/STDERR support

Initial Comment:
At this moment STDOUT and STDERR are used to get
messages into the JmolConsole, making it rather
difficult to catch and redirect these somewhere other
than this JmolConsole. For example, Bioclipse has it's
own console, but some Jmol messages are debug or error
messages, while others are really informative messages
for the user. Moreover, I can't just route STDOUT to
the BioclipseConsole anyway, as any Bioclipse plugin
could potentially spit things to STDOUT...

----------------------------------------------------------------------

>Comment By: Nicolas (nicove)
Date: 2006-07-13 23:36

Message:
Logged In: YES 
user_id=1096197

I have just done the first version of the Logger, but SVN
commit isn't currently working. I will try to commit it
tomorrow.

I have created 5 levels, and I suggest to use them this way:
- fatal: fatal errors in Jmol (error + tells that Jmol may
not work correctly afterwards)
- error: errors in Jmol (unexpected error: indicates a
coding problem)
- warn: user error (wrong script, ...) or problem that is
dealt with by Jmol (incorrect file format, ...)
- info: information only
- debug (deactivated by default): for developers

For other applications integrating Jmol (Bioclipse for
example), the default logger can be replaced by a custom logger.

Each level can be activated/deactivated through a system
property "jmol.logger.<level>" or by calling
Logger.setActiveLevel(). It works even if a custom logger is
used.

Logging is done simply by calling Logger.debug("Text"),
Logger.info("Text"), ...

----------------------------------------------------------------------

Comment By: Bob Hanson (hansonr)
Date: 2006-07-13 17:13

Message:
Logged In: YES 
user_id=1082841

This is an easy addition, at least at the "back end" -- the
class itself and the interface. Nico, do you want to make
the addition? I can go through the Viewer methods after that
and make a call on what's which level. Let's then have a
verbose level that corresponds to these. I can add that to
eval afterwards. 

Please add it to bob200603 anytime if you want to.

----------------------------------------------------------------------

Comment By: Nicolas (nicove)
Date: 2006-07-13 16:13

Message:
Logged In: YES 
user_id=1096197

Let's keep the usual levels in order of importance (Log4j) :
- debug
- info
- warn
- error
- fatal

----------------------------------------------------------------------

Comment By: Bob Hanson (hansonr)
Date: 2006-07-13 15:48

Message:
Logged In: YES 
user_id=1082841

Nico,

OK, that's slick. Can you synthesize these two ideas, adding
in the set enabled/disabled business? I think there is a
middle level of "note" or "warn" between "debug" and "err"
that could be useful. -- Messages that are standard or
informational but could be avoided. Maybe "inform"

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2006-07-13 15:33

Message:
Logged In: NO 

And the DefaultLogger can this have the 'enabled' field.
This is more general and flexible indeed.

BTW, who said we needed a mailing list ?? :)

----------------------------------------------------------------------

Comment By: Nicolas (nicove)
Date: 2006-07-13 15:29

Message:
Logged In: YES 
user_id=1096197

This is really not what I had in mind: I was thinking of
something more flexible.

See below for coding example (only debug and error levels
done, info, warn and fatal could be added)

-------------------

public Interface LoggerInterface {
  public void debug(String txt);
  public void error(String txt);
}

-------------------

public Class DefaultLogger implements LoggerInterface {
  public DefaultLogger() {
  }
  public void debug(String txt) {
    System.out.println(txt);
  }
  public void error(String txt) {
    System.err.println(txt);
  }
}

-------------------

public class Logger {
  private static LoggerInterface _logger = new DefaultLogger();

  public static void setLogger(LoggerInterface logger) {
    _logger = logger;
  }

  public static void debug(String txt) {
    _logger.debug(txt);
  }
  public static void error(String txt) {
    _logger.error(txt);
  }
}

-------------------

Eventually, DefaultLogger can be modified to authorize some
customisation.

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2006-07-13 15:27

Message:
Logged In: NO 

You are correct: I should not have stated this.out here, 
but the local out *is* static, so it should have stated:

  public static void setPrintStream(PrintStream out) {
    Logger.out = out;
  }

----------------------------------------------------------------------

Comment By: Bob Hanson (hansonr)
Date: 2006-07-13 15:25

Message:
Logged In: YES 
user_id=1082841

Ah, I see. That's interesting -- right -- this is the level
of Java I'm not familiar with. I'd like to keep this static
so it can also be used from any static method the way
System.out.println can. How would we do it without using
"this" ?

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2006-07-13 14:49

Message:
Logged In: NO 

It would be nice if one could set the PrintStream too, for 
example:

package org.jmol.util;

public class Logger {

  static boolean systemOutEnabled = true;
  private static PrintStream out = System.out; // the 
default

  public static void setPrintStream(PrintStream out) {
    this.out = out;
  }

  public static void setEnabled(boolean TF) {
    systemOutEnabled = (TF ? Boolean.TRUE : 
Boolean.FALSE);
    out.println("systemOutEnabled = " + TF);
  }

  public static error(String msg) {
    print("ERROR ");
    println(msg);
  }

  private static void println(String msg) {
    if (systemOutEnabled.booleanValue())
      System.out.println(msg);
    }
  }
}

----------------------------------------------------------------------

Comment By: Bob Hanson (hansonr)
Date: 2006-07-13 14:42

Message:
Logged In: YES 
user_id=1082841

OK, how about this. util is a handy class with not too much
in it. Logger seems to me to be basically a utility method.
Could it fit there?

package org.jmol.util;

public class Logger {
  static Boolean systemOutEnabled = Boolean.TRUE;

  public static void setEnabled(boolean TF) {
    systemOutEnabled = (TF ? Boolean.TRUE : Boolean.FALSE);
    out.println("systemOutEnabled = " + TF);
  }

  public static class out {
    public static void println(String msg) {
      if (systemOutEnabled.booleanValue())
        System.out.println(msg);
    }
  }
}

so one can use

Logger.out.println()

instead of System.out.println()

and

Logger.setEnabled(true|false);

and, of course, we could add others. 

OK, you all know better than I how to do this correctly.

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2006-07-13 09:16

Message:
Logged In: NO 

Nico, sounds reasonable.

----------------------------------------------------------------------

Comment By: Nicolas (nicove)
Date: 2006-07-13 08:03

Message:
Logged In: YES 
user_id=1096197

My 2 cts:
- create an Interface (LoggerInterface) with methods:
debug(), info(), warn(), error(), fatal() (the usual levels
for example in Log4j)
- create a Class (DefaultLogger) implementing the above
interface which would simply output messages to stdout or
stderr depending on the level
- create a Class (Logger ?) with static methods debug(), ...
to do the output by calling the above class, and also static
methods to change the class really used for the output.
- replace all the print() calls by Logger.debug(),
Logger.info(), ...

Note: Put all the classes / interfaces above in a separate
package

This would be very adaptable:
- for example, Bioclipse could code it's own class
implementing LoggerInterface and use it instead of Jmol
default one.
- if Jmol is integrated in an other project, they could for
example use Log4J

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2006-07-13 07:20

Message:
Logged In: NO 

Ok, the systemOutPrintln() is probably a first good step; 
splitting them up according to some verbosity level would 
be good too. Except for the verbosity level, I would like 
to request the option to be able to set the PrintStream to 
where the "sysout"s go.

----------------------------------------------------------------------

Comment By: Bob Hanson (hansonr)
Date: 2006-07-13 07:12

Message:
Logged In: YES 
user_id=1082841

There isn't an actual request here, but I did some testing,
and I have this to report:

There are 412 instances of "System.out.println" in the
Viewer package. Some of these might be commented out; it's
not a perfect count.

I changed these to "viewer.systemOutPrintln" calls, and then
added a flag that turns these off. "set systemOutEnabled
false" and back on "set systemOutEnabled true". 

Interestingly enough, there was also an unused nonpublic
"logError()" function that did exactly this, but was never
implemented. So basically I have replaced
"viewer.logError()" with "viewer.systemOutPrintln()".

I realize that that is only the first step - the second
would be to go through these and decide which should go to
the equivalent of STDOUT and which to the equivalent of
STDERR -- and which should just be abandoned. But the point
is, it's pretty easy to do this sort of wholesale change. 

Should I commit these changes? Are we OK with that? It
obviously doesn't affect any functionality (well, it COULD,
but I think we've been careful not to load any actual
functionality into any System.out.println() call. So I think
the chances are very good that it is not a problem. 

The changes are all minor, but they are all over the place
(about 70 files, I think).

And the changes I put in really make it SILENT when you flip
that toggle. We could have something like verbose levels if
we wanted. 

You decide. What's the actual feature request?

Bob

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=379136&aid=1521473&group_id=23629

[Jmol-developers] [ jmol-Feature Requests-1521473 ] Better STDOUT/STDERR support

An interactive viewer for three-dimensional chemical structures.

[Jmol-developers] [ jmol-Feature Requests-1521473 ] Better STDOUT/STDERR support