Re: *print-right-margin* ineffective for write

[Charles Turner <chturne at gmail dot com>]

On 30/11/11 22:49, Charles Turner wrote:
> On 30/11/11 20:13, Taylor Venable wrote:
> > Hi there. I've hit what looks like it might be a problem with 
> > *print-right-margin* and display vs write. 
>
> Yep, that's definitely a problem. For reasons currently unknown to me.
The reasons are now clear :-). What's happening is that instead of 
starting a list print calling PrettyWriter#startLogicalBlock, WRITE 
starts with PrettyWriter#writePositionMarker. This is an SRFI-38 
specific thing, which DISPLAY is oblivious to. 
PrettyWriter#writePositionMarker wasn't checking the new printing 
environment, so when that check occurred in startLogicalBlock, there was 
already a QITEM_POSNMARKER in the queue (due to writePositionMarker 
being called first), hence the invariant was broken.

A solution is to factor the <check printing environment> stuff out of 
startLogicalBlock and call it from writePositionMarker. This isn't very 
efficient, since writePosnMarker is called a *lot* for large lists, and 
most of the time the check is an unnecessary burden on the call stack. 
The alternative is to check in startLogicalBlock (a much less frequently 
called method) whether queueSize is <= <size of a posn marker>. This 
isn't very robust, though, since something other than a posn marker 
might be in the queue before this check is made, and then we might be in 
trouble.

So, the attached will most likely need some comments before being 
checked in, but hopefully it will solve your problem. Please let me know 
if it does, sorry the inconvenience.

Thanks,

Charlie.
Index: gnu/text/PrettyWriter.java
===================================================================
--- gnu/text/PrettyWriter.java	(revision 7087)
+++ gnu/text/PrettyWriter.java	(working copy)
@@ -673,6 +673,9 @@
    */
   public int writePositionMarker (boolean grouping)
   {
+    // When checking for shared structures, this queue item will be the first
+    // to be written. A good time to check the current pretty printing environment.
+    checkPrintMargins();
     int result = enqueue(QITEM_POSNMARKER_TYPE, QITEM_POSNMARKER_SIZE);
     // A zero implies we haven't assigned this position marker a count yet
     queueInts[result + QITEM_POSNMARKER_LOC] = 0;
@@ -1005,11 +1008,12 @@
 		     : QITEM_INDENTATION_BLOCK),
 		    amount);
   }
-
-  public void startLogicalBlock (String prefix, boolean perLine, String suffix)
-  {
-    // If the queue is empty, it is a good time to check if line-length etc
-    // have been changed.
+  
+  /**
+   * If we're about to start a new print, check the current environment
+   * for new pretty printing parameters.
+   */
+  private void checkPrintMargins() {
     if (queueSize == 0 && bufferFillPointer == 0)
       {
         Object llen = lineLengthLoc.get(null);
@@ -1026,9 +1030,16 @@
         else
           miserWidth = Integer.parseInt(mwidth.toString());
 
-        Object indent = indentLoc.get(null);
-        // if (indent == null || indent ...
+        // TODO: Currently not used.
+        // Object indent = indentLoc.get(null);
       }
+  }
+
+  public void startLogicalBlock (String prefix, boolean perLine, String suffix)
+  {
+    // If the queue is empty, it is a good time to check if line-length etc
+    // have been changed.
+    checkPrintMargins();
     if (prefix != null)
       write(prefix);
     if (prettyPrintingMode == 0)