public inbox for frysk@sourceware.org
 help / color / mirror / Atom feed
* Patch: word-wrapping for 'help' output
@ 2008-02-29 21:18 Tom Tromey
  2008-03-01 22:04 ` Mark Wielaard
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2008-02-29 21:18 UTC (permalink / raw)
  To: Frysk List

This patch implements word-wrapping for frysk 'help' output.

The idea is simple: when writing help output, write through a new
PrintWriter which handles word-wrapping.  This patch goes a bit beyond
that, though, and provides some nice formatting in some particular
cases.  For instance, when printing an option, if the option's
description wraps, then the wrapped line will be indented.  This makes
it nicer to read.

I did not try to deal with letting the user configure the number of
columns.  However the wrapping code is general and ought to handle
future changes here gracefully.  Likewise, currently the code only
handles the default locale, but again is written to upgrade easily.

Tom

frysk-core/frysk/hpd/ChangeLog:
2008-02-29  Tom Tromey  <tromey@redhat.com>

	* 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  <tromey@redhat.com>

	* WordWrapWriter.java: New file.

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);
+    }
+
     final PrintWriter outWriter;
     private Preprocessor prepro;
     private String prompt; // string to represent prompt, will be moved
diff --git a/frysk-core/frysk/hpd/Command.java b/frysk-core/frysk/hpd/Command.java
index ecbb374..3060b9a 100644
--- a/frysk-core/frysk/hpd/Command.java
+++ b/frysk-core/frysk/hpd/Command.java
@@ -1,6 +1,6 @@
 // This file is part of the program FRYSK.
 //
-// Copyright 2007, Red Hat Inc.
+// Copyright 2007, 2008, Red Hat Inc.
 //
 // FRYSK is free software; you can redistribute it and/or modify it
 // under the terms of the GNU General Public License as published by
@@ -40,6 +40,7 @@
 package frysk.hpd;
 
 import java.util.List;
+import java.io.PrintWriter;
 
 /**
  * A handler class for the CLI that supplies its own help messages.
@@ -72,11 +73,12 @@ public abstract class Command {
      * command.
      */
     void help(CLI cli, Input buffer) {
-	cli.outWriter.print("Usage: ");
-	cli.outWriter.print(syntax);
-	cli.outWriter.println();
-	cli.outWriter.print(full);
-	cli.outWriter.println();
+	PrintWriter out = cli.getWordWrapWriter();
+	out.print("Usage: ");
+	out.print(syntax);
+	out.println();
+	out.print(full);
+	out.println();
     }
 
     /**
diff --git a/frysk-core/frysk/hpd/MultiLevelCommand.java b/frysk-core/frysk/hpd/MultiLevelCommand.java
index 266d5ef..ac51455 100644
--- a/frysk-core/frysk/hpd/MultiLevelCommand.java
+++ b/frysk-core/frysk/hpd/MultiLevelCommand.java
@@ -1,6 +1,6 @@
 // This file is part of the program FRYSK.
 //
-// Copyright 2007, Red Hat Inc.
+// Copyright 2007, 2008, Red Hat Inc.
 //
 // FRYSK is free software; you can redistribute it and/or modify it
 // under the terms of the GNU General Public License as published by
@@ -45,6 +45,7 @@ import java.util.HashMap;
 import java.util.TreeMap;
 import java.util.Iterator;
 import java.util.List;
+import frysk.util.WordWrapWriter;
 
 /**
  * A handler class for the CLI that supplies its own help messages.
@@ -115,14 +116,17 @@ public abstract class MultiLevelCommand extends Command {
      */
     void help(CLI cli, Input input) {
 	if (input.size() == 0) {
+	    WordWrapWriter out = cli.getWordWrapWriter();
 	    for (Iterator i = subCommands.entrySet().iterator();
 		 i.hasNext(); ) {
 		Map.Entry entry = (Map.Entry)(i.next());
 		String name = (String)entry.getKey();
 		Command command = (Command)entry.getValue();
-		cli.outWriter.print(name);
-		cli.outWriter.print(" - ");
-		cli.outWriter.println(command.description());
+		out.print(name);
+		out.print(" - ");
+		out.setWrapIndentFromColumn();
+		out.println(command.description());
+		out.setWrapIndent(0);
 	    }
 	} else {
 	    lookup(input.parameter(0)).help(cli, input.accept());
diff --git a/frysk-core/frysk/hpd/NoOptsCommand.java b/frysk-core/frysk/hpd/NoOptsCommand.java
index 7edc566..b37ce14 100644
--- a/frysk-core/frysk/hpd/NoOptsCommand.java
+++ b/frysk-core/frysk/hpd/NoOptsCommand.java
@@ -1,6 +1,6 @@
 // This file is part of the program FRYSK.
 //
-// Copyright 2007, Red Hat Inc.
+// Copyright 2007, 2008, Red Hat Inc.
 //
 // FRYSK is free software; you can redistribute it and/or modify it
 // under the terms of the GNU General Public License as published by
@@ -39,6 +39,8 @@
 
 package frysk.hpd;
 
+import java.io.PrintWriter;
+
 /**
  * Handle commands that should not interpret options.
  */
@@ -72,8 +74,9 @@ public abstract class NoOptsCommand extends Command {
     abstract void interpretCommand(CLI cli, Input input);
     
     void help(CLI cli, Input input) {
-	cli.outWriter.print(syntax);
-	cli.outWriter.println();
-	cli.outWriter.println(full);
+	PrintWriter out = cli.getWordWrapWriter();
+	out.print(syntax);
+	out.println();
+	out.println(full);
     }
 }
\ No newline at end of file
diff --git a/frysk-core/frysk/hpd/ParameterizedCommand.java b/frysk-core/frysk/hpd/ParameterizedCommand.java
index 35ec6a6..99bfe7c 100644
--- a/frysk-core/frysk/hpd/ParameterizedCommand.java
+++ b/frysk-core/frysk/hpd/ParameterizedCommand.java
@@ -1,6 +1,6 @@
 // This file is part of the program FRYSK.
 //
-// Copyright 2007, Red Hat Inc.
+// Copyright 2007, 2008, Red Hat Inc.
 //
 // FRYSK is free software; you can redistribute it and/or modify it
 // under the terms of the GNU General Public License as published by
@@ -43,6 +43,7 @@ import java.util.TreeMap;
 import java.util.Iterator;
 import java.util.SortedMap;
 import java.util.List;
+import frysk.util.WordWrapWriter;
 
 abstract class ParameterizedCommand extends Command {
     private final SortedMap longOptions = new TreeMap();
@@ -147,22 +148,25 @@ abstract class ParameterizedCommand extends Command {
     }
 
     void help(CLI cli, Input input) {
-	cli.outWriter.print(syntax);
+	WordWrapWriter out = cli.getWordWrapWriter();
+	out.print(syntax);
 	if (longOptions.size() > 0) {
-	    cli.outWriter.println(" -option ...; where options are:");
+	    out.println(" -option ...; where options are:");
 	    for (Iterator i = longOptions.values().iterator();
 		 i.hasNext(); ) {
 		CommandOption option = (CommandOption)i.next();
-		cli.outWriter.print("  -");
-		cli.outWriter.print(option.longName);
-		cli.outWriter.print("\t");
-		cli.outWriter.print(option.description);
-		cli.outWriter.println();
+		out.print("  -");
+		out.print(option.longName);
+		out.print("\t");
+		out.setWrapIndentFromColumn();
+		out.print(option.description);
+		out.setWrapIndent(0);
+		out.println();
 	    }
 	} else {
-	    cli.outWriter.println();
+	    out.println();
 	}
-	cli.outWriter.println(full);
+	out.println(full);
     }
 
     /**
diff --git a/frysk-core/frysk/util/WordWrapWriter.java b/frysk-core/frysk/util/WordWrapWriter.java
new file mode 100644
index 0000000..5aa8527
--- /dev/null
+++ b/frysk-core/frysk/util/WordWrapWriter.java
@@ -0,0 +1,166 @@
+// This file is part of the program FRYSK.
+//
+// Copyright 2008 Red Hat Inc.
+//
+// FRYSK is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License as published by
+// the Free Software Foundation; version 2 of the License.
+//
+// FRYSK is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+// General Public License for more details.
+// 
+// You should have received a copy of the GNU General Public License
+// along with FRYSK; if not, write to the Free Software Foundation,
+// Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+// 
+// In addition, as a special exception, Red Hat, Inc. gives You the
+// additional right to link the code of FRYSK with code not covered
+// under the GNU General Public License ("Non-GPL Code") and to
+// distribute linked combinations including the two, subject to the
+// limitations in this paragraph. Non-GPL Code permitted under this
+// exception must only link to the code of FRYSK through those well
+// defined interfaces identified in the file named EXCEPTION found in
+// the source code files (the "Approved Interfaces"). The files of
+// Non-GPL Code may instantiate templates or use macros or inline
+// functions from the Approved Interfaces without causing the
+// resulting work to be covered by the GNU General Public
+// License. Only Red Hat, Inc. may make changes or additions to the
+// list of Approved Interfaces. You must obey the GNU General Public
+// License in all respects for all of the FRYSK code and other code
+// used in conjunction with FRYSK except the Non-GPL Code covered by
+// this exception. If you modify this file, you may extend this
+// exception to your version of the file, but you are not obligated to
+// do so. If you do not wish to provide this exception without
+// modification, you must delete this exception statement from your
+// version and license this file solely under the GPL without
+// exception.
+
+package frysk.util;
+
+import java.io.PrintWriter;
+import java.io.Writer;
+import java.util.Locale;
+import java.text.BreakIterator;
+import java.text.StringCharacterIterator;
+
+/**
+ * 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.
+ */
+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());
+    }
+
+    public void setColumns(int columns) {
+	this.columns = columns;
+    }
+
+    public void setWrapIndent(int wrapIndent) {
+	this.wrapIndent = wrapIndent;
+    }
+
+    public void setWrapIndentFromColumn() {
+	this.wrapIndent = this.column;
+    }
+
+    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));
+    }
+
+    // Update 'column' assuming STR is printed.  We use a helper
+    // function here to properly handle tabs.
+    private void updateColumn(String str) {
+	int len = str.length();
+	for (int i = 0; i < len; ++i) {
+	    if (str.charAt(i) == '\t') {
+		column = 8 * ((column + 8) / 8);
+	    } else {
+		++column;
+	    }
+	}
+    }
+
+    public void write(String str, int offset, int len) {
+	int term = offset + len;
+	while (offset < term) {
+	    // Find the next newline, if there is one.
+	    int nlIndex = str.indexOf('\n', offset);
+	    if (nlIndex >= term)
+		nlIndex = -1;
+	    // Only operate on the current line.  If we don't see a
+	    // \n, operate on all the remaining text.
+	    int subLen = (nlIndex < 0) ? len : (nlIndex - offset + 1);
+	    iter.setText(new StringCharacterIterator(str, offset,
+						     offset + subLen, offset));
+	    int start = iter.first();
+	    int end = iter.next();
+	    boolean first = column == 0;
+	    while (end != BreakIterator.DONE) {
+		String word = str.substring(start, end);
+		updateColumn(word);
+		if (!first && column >= columns) {
+		    super.write('\n');
+		    for (int i = 0; i < wrapIndent; ++i)
+			super.write(' ');
+		    column = wrapIndent;
+		    // The first word on a line should not start with
+		    // a space.  We use '<=' to work like String.trim,
+		    // and to eliminate tabs as well.
+		    int j = 0;
+		    while (j < word.length() && word.charAt(j) <= ' ')
+			++j;
+		    if (j > 0)
+			word = word.substring(j);
+		    updateColumn(word);
+		}
+		first = false;
+		// Avoid recursion here...
+		super.write(word, 0, word.length());
+		start = end;
+		end = iter.next();
+	    }
+
+	    if (nlIndex >= 0) {
+		// We already wrote the \n.
+		column = 0;
+	    }
+
+	    offset += subLen;
+	    len -= subLen;
+	}
+    }
+}

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch: word-wrapping for 'help' output
  2008-02-29 21:18 Patch: word-wrapping for 'help' output Tom Tromey
@ 2008-03-01 22:04 ` Mark Wielaard
  2008-03-02  0:17   ` Tom Tromey
  2008-03-02  0:42   ` Tom Tromey
  0 siblings, 2 replies; 8+ messages in thread
From: Mark Wielaard @ 2008-03-01 22:04 UTC (permalink / raw)
  To: tromey; +Cc: Frysk List

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  <tromey@redhat.com>
> 
> 	* 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  <tromey@redhat.com>
> 
> 	* 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch: word-wrapping for 'help' output
  2008-03-01 22:04 ` Mark Wielaard
@ 2008-03-02  0:17   ` Tom Tromey
  2008-03-02 12:34     ` Mark Wielaard
  2008-03-02  0:42   ` Tom Tromey
  1 sibling, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2008-03-02  0:17 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Frysk List

Mark> This could use a method description.

You'll forgive me for assuming that the frysk style was not to write
javadoc.

Here's an add-on patch that adds javadoc.  Let me know what you think.
I guess I should learn git enough to supply consolidated patches.

Mark> The CLI doesn't have the PtyTerminal (created in fhpd.java).
Mark> It might make sense to pass that to the CLI so it can use
Mark> getTerminalWidth() here when constructing the WordWrapWriter.

I will look at this shortly.

Tom

b/frysk-core/frysk/hpd/ChangeLog:
2008-03-01  Tom Tromey  <tromey@redhat.com>

	* CLI.java (getWordWrapWriter): Document.

b/frysk-core/frysk/util/ChangeLog:
2008-03-01  Tom Tromey  <tromey@redhat.com>

	* WordWrapWriter.java: Document.

diff --git a/frysk-core/frysk/hpd/CLI.java b/frysk-core/frysk/hpd/CLI.java
index 67b3788..f8f16c8 100644
--- a/frysk-core/frysk/hpd/CLI.java
+++ b/frysk-core/frysk/hpd/CLI.java
@@ -182,6 +182,9 @@ public class CLI {
             idManager.manageProc(proc, this.taskID);
     }
 
+    /**
+     * Return a WordWrapWriter which wraps this CLI's output writer.
+     */
     WordWrapWriter getWordWrapWriter() {
 	return new WordWrapWriter(outWriter);
     }
diff --git a/frysk-core/frysk/util/WordWrapWriter.java b/frysk-core/frysk/util/WordWrapWriter.java
index 5aa8527..e8da436 100644
--- a/frysk-core/frysk/util/WordWrapWriter.java
+++ b/frysk-core/frysk/util/WordWrapWriter.java
@@ -64,6 +64,13 @@ public class WordWrapWriter extends PrintWriter {
     // The current column.
     private int column;
 
+    /**
+     * Create a new WordWrapWriter, specifying all parameters.
+     * @param outStream the output writer to wrap
+     * @param columns the number of columns to allow before wrapping
+     * @param wrapIndent the number of columns to indent after wrapping
+     * @param locale the locale to use for determining word breaks
+     */
     public WordWrapWriter(Writer outStream, int columns, int wrapIndent, Locale locale) {
 	// Always enable auto-flush.
 	super(outStream, true);
@@ -72,26 +79,64 @@ public class WordWrapWriter extends PrintWriter {
 	this.iter = BreakIterator.getWordInstance(locale);
     }
 
+    /**
+     * Create a new WordWrapWriter, specifying just the number of columns.
+     * By default there will be no indentation after a wrap, and the
+     * default locale will be used.
+     * @param outStream the output writer to wrap
+     * @param columns the number of columns to allow before wrapping
+     */
     public WordWrapWriter(Writer outStream, int columns) {
 	this(outStream, columns, 0, Locale.getDefault());
     }
 
+    /**
+     * Create a new WordWrapWriter using the defaults.  Wrapping will
+     * happen at column 72.  By default there will be no indentation
+     * after a wrap, and the default locale will be used.
+     * @param outStream the output writer to wrap
+     */
     public WordWrapWriter(Writer outStream) {
 	this(outStream, 72, 0, Locale.getDefault());
     }
 
+    /**
+     * Set the number of columns of output.  The writer will try to
+     * break a line before a word that would go past this column.
+     * @param columns the number of columns to allow before wrapping
+     */
     public void setColumns(int columns) {
 	this.columns = columns;
     }
 
+    /**
+     * Set the amount of indentation after wrapping.  This can be used
+     * to line up some text if it wraps past the end of the line.
+     * Indentation is accomplished by emitting spaces.  Tabs in the
+     * output are considered to move to the next column that is a
+     * multiple of 8 ("unix style").  An argument of 0 means that no
+     * indentation will be applied after wrapping.
+     * @param wrapIndent the number of columns to indent after wrapping
+     */
     public void setWrapIndent(int wrapIndent) {
 	this.wrapIndent = wrapIndent;
     }
 
+    /**
+     * Like setWrapIndent(int), but sets the indentation column based
+     * on the current column known to this writer.  This can be useful
+     * for aligning text when the precise formatting is not known --
+     * you can emit a prefix for a line, mark the indentation level,
+     * and then emit the remainder of the text, which will all line up
+     * at the marked position.
+     */
     public void setWrapIndentFromColumn() {
 	this.wrapIndent = this.column;
     }
 
+    // All PrintWriter output methods (in particular the print
+    // methods) eventually delegate to a write() method.  We override
+    // just the necessary ones to have everything work properly.
     public void write(char[] buf, int offset, int len) {
 	// A bit inefficient but we don't care much.
 	write(new String(buf, offset, len));

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch: word-wrapping for 'help' output
  2008-03-01 22:04 ` Mark Wielaard
  2008-03-02  0:17   ` Tom Tromey
@ 2008-03-02  0:42   ` Tom Tromey
  2008-03-02 12:48     ` Mark Wielaard
  1 sibling, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2008-03-02  0:42 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Frysk List

Mark> The CLI doesn't have the PtyTerminal (created in fhpd.java).
Mark> It might make sense to pass that to the CLI so it can use
Mark> getTerminalWidth() here when constructing the WordWrapWriter.

Here's a patch to do it -- but *don't* check this in.

getTerminalWidth seems to return something bogus, because everything
started wrapping at column 20.  I didn't look into this any more
deeply.

Tom

b/frysk-core/frysk/bindir/ChangeLog:
2008-03-01  Tom Tromey  <tromey@redhat.com>

	* fhpd.java (CommandLine): Pass terminal to CLI constructor.

b/frysk-core/frysk/hpd/ChangeLog:
2008-03-01  Tom Tromey  <tromey@redhat.com>

	* CLI.java (terminal): New field.
	(CLI): Add 'terminal' argument.
	(CLI(String,ObservingTerminal,Writer)): New constructor.
	(getWordWrapWriter): Compute number of columns.

diff --git a/frysk-core/frysk/bindir/fhpd.java b/frysk-core/frysk/bindir/fhpd.java
index efc8520..69be24c 100644
--- a/frysk-core/frysk/bindir/fhpd.java
+++ b/frysk-core/frysk/bindir/fhpd.java
@@ -106,7 +106,7 @@ public class fhpd {
                 FlowControlWriter writer = new FlowControlWriter(printWriter);
                 terminal.getObservable()
                     .addObserver(new TerminalObserver(writer));
-                cli = new CLI("(fhpd) ", writer);
+                cli = new CLI("(fhpd) ", terminal, writer);
 		reader = new ConsoleReader
                     (new FileInputStream(java.io.FileDescriptor.in),
 		     printWriter,
diff --git a/frysk-core/frysk/hpd/CLI.java b/frysk-core/frysk/hpd/CLI.java
index f8f16c8..08e8ed6 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.ObservingTerminal;
 import frysk.util.WordWrapWriter;
 import frysk.expr.Expression;
 import frysk.expr.ScratchSymTab;
@@ -186,10 +187,17 @@ public class CLI {
      * Return a WordWrapWriter which wraps this CLI's output writer.
      */
     WordWrapWriter getWordWrapWriter() {
-	return new WordWrapWriter(outWriter);
+	// If there is no terminal, use the standard.
+	int cols = terminal == null ? 80 : terminal.getTerminalWidth();
+	// Subtract a bit so that the words don't run right up to the
+	// terminal's edge.  But, if the user makes a very silly
+	// terminal size, just pick something a bit less weird.
+	cols = Math.max(20, cols - 8);
+	return new WordWrapWriter(outWriter, cols);
     }
 
     final PrintWriter outWriter;
+    final ObservingTerminal terminal;
     private Preprocessor prepro;
     private String prompt; // string to represent prompt, will be moved
     private final Command topLevelCommand = new TopLevelCommand();
@@ -226,11 +234,13 @@ public class CLI {
     /**
      * Constructor
      * @param prompt String initially to be used as the prompt
+     * @param terminal the terminal to which we are connected
      * @param out PrintWriter for output
      * @param steppingEngine existing SteppingEngine
      */
-    public CLI(String prompt, Writer outWriter, SteppingEngine steppingEngine) {
+    public CLI(String prompt, ObservingTerminal terminal, Writer outWriter, SteppingEngine steppingEngine) {
         this.prompt = prompt;
+	this.terminal = terminal;
         this.outWriter = new PrintWriter(outWriter);
         this.steppingEngine = steppingEngine;
         idManager = ProcTaskIDManager.getSingleton();
@@ -262,10 +272,20 @@ public class CLI {
     /**
      * Constructor that creates a new steppingEngine
      * @param prompt String initially to be used as the prompt
+     * @param terminal the terminal to which we are connected
+     * @param out PrintWriter for output.
+     */
+    public CLI(String prompt, ObservingTerminal terminal, Writer outWriter) {
+        this(prompt, terminal, outWriter, new SteppingEngine());
+    }
+   
+    /**
+     * Constructor that creates a new steppingEngine
+     * @param prompt String initially to be used as the prompt
      * @param out PrintWriter for output.
      */
     public CLI(String prompt, Writer outWriter) {
-        this(prompt, outWriter, new SteppingEngine());
+        this(prompt, null, outWriter, new SteppingEngine());
     }
    
     public String getPrompt() {

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch: word-wrapping for 'help' output
  2008-03-02  0:17   ` Tom Tromey
@ 2008-03-02 12:34     ` Mark Wielaard
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Wielaard @ 2008-03-02 12:34 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Frysk List

Hi Tom,

On Sat, 2008-03-01 at 16:26 -0700, Tom Tromey wrote:
> Mark> This could use a method description.
> 
> You'll forgive me for assuming that the frysk style was not to write
> javadoc.

Documentation is really important. Especially later on when you look bad
at code you might not have written yourself and try to figure out what
and why it actually works. In this case I should probably have added it
myself since I was reviewing the code anyway. Sorry I was a bit lazy.
And thanks for picking it up!

> Here's an add-on patch that adds javadoc.  Let me know what you think.
> I guess I should learn git enough to supply consolidated patches.

This looks great! Don't worry too much about consolidating the patches.
I'll commit them separately on top of each other and then push them in
one go. It only really matter if the intermediate commits are bad (so
you cannot bisect through them when searching when something broke).
Some more git info and references can be found at:
http://sourceware.org/frysk/build/git-fu.html

> b/frysk-core/frysk/hpd/ChangeLog:
> 2008-03-01  Tom Tromey  <tromey@redhat.com>
> 
> 	* CLI.java (getWordWrapWriter): Document.
> 
> b/frysk-core/frysk/util/ChangeLog:
> 2008-03-01  Tom Tromey  <tromey@redhat.com>
> 
> 	* WordWrapWriter.java: Document.

Committed and will be pushed soon.

Thanks,

Mark

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch: word-wrapping for 'help' output
  2008-03-02  0:42   ` Tom Tromey
@ 2008-03-02 12:48     ` Mark Wielaard
  2008-03-02 16:10       ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Wielaard @ 2008-03-02 12:48 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Frysk List

Hi Tom,

On Sat, 2008-03-01 at 16:51 -0700, Tom Tromey wrote:
> Mark> The CLI doesn't have the PtyTerminal (created in fhpd.java).
> Mark> It might make sense to pass that to the CLI so it can use
> Mark> getTerminalWidth() here when constructing the WordWrapWriter.
> 
> Here's a patch to do it -- but *don't* check this in.
> 
> getTerminalWidth seems to return something bogus, because everything
> started wrapping at column 20.  I didn't look into this any more
> deeply.

Hmmm. Might be a bug in jline I guess.
I filed a bug for this issue pointing to this patch.
http://sourceware.org/bugzilla/show_bug.cgi?id=5817

And a separate one for upgrading our jline in frysk-imports.
http://sourceware.org/bugzilla/show_bug.cgi?id=5816

Hopefully it contains a fix. I didn't check though. We are still using a
fairly old 0.6 release of jline, while current upstream is 0.9.3. Tim
might know the story. I believe a newer version didn't compile cleanly
and had to be reverted just before we pulled from git for a fedora
release.

Cheers,

Mark

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch: word-wrapping for 'help' output
  2008-03-02 12:48     ` Mark Wielaard
@ 2008-03-02 16:10       ` Tom Tromey
  2008-03-02 22:05         ` Mark Wielaard
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2008-03-02 16:10 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Frysk List

>>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:

Mark> Hmmm. Might be a bug in jline I guess.
Mark> I filed a bug for this issue pointing to this patch.
Mark> http://sourceware.org/bugzilla/show_bug.cgi?id=5817

I forgot to mention that I did see a comment in PtyTerminal saying
that getTerminalWidth caches the result -- oops, that is bad.  At the
very least it should be clearing the cache on SIGWINCH.

I looked at the code a bit more deeply, and it just runs 'stty size'.
It actually looks ok to me, so I don't know what is wrong there.

BTW, PtyTerminal.stty uses Runtime.exec.  I thought that was a no-no
in frysk.

Tom

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Patch: word-wrapping for 'help' output
  2008-03-02 16:10       ` Tom Tromey
@ 2008-03-02 22:05         ` Mark Wielaard
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Wielaard @ 2008-03-02 22:05 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Frysk List

On Sun, 2008-03-02 at 08:19 -0700, Tom Tromey wrote:
> I looked at the code a bit more deeply, and it just runs 'stty size'.
> It actually looks ok to me, so I don't know what is wrong there.
> 
> BTW, PtyTerminal.stty uses Runtime.exec.  I thought that was a no-no
> in frysk.

That might indeed be problematic. Since Runtime will try to reap
processes by calling wait() and frysk wants to receive all waits itself.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2008-03-02 22:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-29 21:18 Patch: word-wrapping for 'help' output Tom Tromey
2008-03-01 22:04 ` Mark Wielaard
2008-03-02  0:17   ` Tom Tromey
2008-03-02 12:34     ` Mark Wielaard
2008-03-02  0:42   ` Tom Tromey
2008-03-02 12:48     ` Mark Wielaard
2008-03-02 16:10       ` Tom Tromey
2008-03-02 22:05         ` Mark Wielaard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).