From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23386 invoked by alias); 1 Mar 2008 22:04:00 -0000 Received: (qmail 23376 invoked by uid 22791); 1 Mar 2008 22:03:59 -0000 X-Spam-Status: No, hits=-0.3 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_34,J_CHICKENPOX_44,J_CHICKENPOX_47,J_CHICKENPOX_53,J_CHICKENPOX_63,J_CHICKENPOX_74 X-Spam-Check-By: sourceware.org Received: from wildebeest.demon.nl (HELO gnu.wildebeest.org) (83.160.170.119) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 01 Mar 2008 22:03:42 +0000 Received: from wildebeest.demon.nl ([83.160.170.119] helo=[127.0.0.1]) by gnu.wildebeest.org with esmtp (Exim 4.63) (envelope-from ) id 1JVZnu-0006ju-NV; Sat, 01 Mar 2008 23:03:39 +0100 Subject: Re: Patch: word-wrapping for 'help' output From: Mark Wielaard To: tromey@redhat.com Cc: Frysk List In-Reply-To: References: Content-Type: text/plain Date: Sat, 01 Mar 2008 22:04:00 -0000 Message-Id: <1204409018.3279.56.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.12.3 (2.12.3-1.fc8) Content-Transfer-Encoding: 7bit X-Spam-Score: -4.0 (----) X-IsSubscribed: yes Mailing-List: contact frysk-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: frysk-owner@sourceware.org X-SW-Source: 2008-q1/txt/msg00097.txt.bz2 Hi Tom, On Fri, 2008-02-29 at 13:27 -0700, Tom Tromey wrote: > This patch implements word-wrapping for frysk 'help' output. Wow, this is really useful! > frysk-core/frysk/hpd/ChangeLog: > 2008-02-29 Tom Tromey > > * ParameterizedCommand.java (help): Use getWordWrapWriter. Set > indent for wrapping. > * NoOptsCommand.java (help): Use getWordWrapWriter. > * MultiLevelCommand.java (help): use getWordWrapWriter. Set > indent for wrapping. > * Command.java (help): Use getWordWrapWriter. > * CLI.java (getWordWrapWriter): New method. > > frysk-core/frysk/util/ChangeLog: > 2008-02-29 Tom Tromey > > * WordWrapWriter.java: New file. Some very minor questions and suggestions. It is pretty good. Also I saw no test regressions. Nice work. > diff --git a/frysk-core/frysk/hpd/CLI.java b/frysk-core/frysk/hpd/CLI.java > index 35bc9e5..67b3788 100644 > --- a/frysk-core/frysk/hpd/CLI.java > +++ b/frysk-core/frysk/hpd/CLI.java > @@ -61,6 +61,7 @@ import frysk.rt.ProcTaskIDManager; > import frysk.stepping.SteppingEngine; > import frysk.stepping.TaskStepEngine; > import frysk.util.CountDownLatch; > +import frysk.util.WordWrapWriter; > import frysk.expr.Expression; > import frysk.expr.ScratchSymTab; > import frysk.expr.ExprSymTab; > @@ -181,6 +182,10 @@ public class CLI { > idManager.manageProc(proc, this.taskID); > } > > + WordWrapWriter getWordWrapWriter() { > + return new WordWrapWriter(outWriter); > + } This could use a method description. The CLI doesn't have the PtyTerminal (created in fhpd.java). It might make sense to pass that to the CLI so it can use getTerminalWidth() here when constructing the WordWrapWriter. > +/** > + * This class is a PrintWriter which word-wraps the output. It > + * provides settings which control over the number of columns and > + * indentation of wrapped lines. > + */ An example would be nice. > +public class WordWrapWriter extends PrintWriter { > + // Number of columns available. > + private int columns; > + > + // Amount of indentation to use on wrapped line. > + private int wrapIndent; > + > + // The break iterator, used to find the next line-break > + // opportunity. > + private BreakIterator iter; > + > + // The current column. > + private int column; > + > + public WordWrapWriter(Writer outStream, int columns, int wrapIndent, Locale locale) { > + // Always enable auto-flush. > + super(outStream, true); > + this.columns = columns; > + this.wrapIndent = wrapIndent; > + this.iter = BreakIterator.getWordInstance(locale); > + } > + > + public WordWrapWriter(Writer outStream, int columns) { > + this(outStream, columns, 0, Locale.getDefault()); > + } > + > + public WordWrapWriter(Writer outStream) { > + this(outStream, 72, 0, Locale.getDefault()); > + } The defaults should really be documented. > + public void setWrapIndentFromColumn() { > + this.wrapIndent = this.column; > + } This could really use a comment. It took me a while to realize why it is useful. > + public void write(char[] buf, int offset, int len) { > + // A bit inefficient but we don't care much. > + write(new String(buf, offset, len)); > + } > + > + public void write(int b) { > + write(String.valueOf((char) b)); > + } A comment about which write methods there are in a PrintWriter would have helped. I actually had to double check to see if you overrode them all and whether the print() methods all delegate to the write() methods. Thanks, Mark