Factor plugin -- better handling of external Factor errors, better error highlighting

TODO.FACTOR.txt

@@ -41,7 +41,6 @@
 - errors don't always disappear
 - console: wrong history
 - listener: if too many things popped off the stack, complain
-- gracefully handle non-working cfactor
 - NPE in ErrorHighlight
 - some way to not have previous definitions from a source file
   clutter the namespace

actions.xml

@@ -11,8 +11,7 @@
 	</ACTION>
 	<ACTION NAME="factor-restart">
 		<CODE>
-			FactorPlugin.stopExternalInstance();
+			FactorPlugin.restartExternalInstance();
-			FactorPlugin.getExternalInstance();
 		</CODE>
 	</ACTION>
 	<ACTION NAME="factor-eval-selection">

build.xml

@@ -24,7 +24,7 @@
 
 	<target name="dist" depends="compile">
 		<jar
-			jarfile="Factor.jar"
+			jarfile="../Factor.jar"
 			compress="true"
 		>
 			<fileset dir=".">
@@ -42,10 +42,6 @@
 		</jar>
 	</target>
 
-	<target name="dist-jedit" depends="dist">
-		<copy file="Factor.jar" tofile="../Factor.jar" />
-	</target>
-
 	<target name="clean" description="Clean old stuff.">
 		<delete>
 			<fileset dir="." includes="**/*.class"/>

examples/factoroids.factor

@@ -323,4 +323,4 @@ SYMBOL: event
 factoroids
 
 ! Currently the plugin doesn't handle GENERIC: and M:, so we
-! disable the parser. too many errors :sidekick.parser=none:
+! disable the parser. too many errors :sidekick.parser=factor:

factor/ExternalFactor.java

@@ -43,8 +43,15 @@ public class ExternalFactor extends DefaultVocabularyLookup
 	/**
 	 * We are given two streams that point to a bare REPL.
 	 */
-	public ExternalFactor(InputStream in, OutputStream out)
+	public ExternalFactor(Process proc, InputStream in, OutputStream out)
-		throws IOException
+	{
+		if(proc == null || in == null || out == null)
+			closed = true;
+		else
+		{
+			this.proc = proc;
+
+			try
 			{
 				this.in = new DataInputStream(in);
 				this.out = new DataOutputStream(out);
@@ -57,6 +64,15 @@ public class ExternalFactor extends DefaultVocabularyLookup
 				/* Start stream server */
 				streamServer = 9999;
 				eval("USE: telnetd [ 9999 telnetd ] in-thread");
+
+				/* Ensure we're ready for a connection immediately */
+				eval("nop");
+			}
+			catch(IOException e)
+			{
+				close();
+			}
+		}
 	} //}}}
 
 	//{{{ waitForAck() method
@@ -90,8 +106,8 @@ public class ExternalFactor extends DefaultVocabularyLookup
 	 */
 	public synchronized String eval(String cmd) throws IOException
 	{
-		/* Log.log(Log.DEBUG,ExternalFactor.class,"SEND: " + cmd); */
+		try
-
+		{
 			waitForAck();
 	
 			sendEval(cmd);
@@ -101,26 +117,48 @@ public class ExternalFactor extends DefaultVocabularyLookup
 			in.readFully(response);
 			
 			String responseStr = new String(response,"ASCII");
-		/* Log.log(Log.DEBUG,ExternalFactor.class,"RECV: " + responseStr); */
 			return responseStr;
+		}
+		catch(IOException e)
+		{
+			close();
+			throw e;
+		}
 	} //}}}
 
 	//{{{ openStream() method
 	/**
 	 * Return a listener stream.
 	 */
-	public FactorStream openStream() throws IOException
+	public FactorStream openStream()
+	{
+		if(closed)
+			return null;
+		else
+		{
+			try
 			{
 				Socket client = new Socket("localhost",streamServer);
 				return new FactorStream(client);
+			}
+			catch(Exception e)
+			{
+				Log.log(Log.ERROR,this,"Cannot open stream connection to "
+					+ "external Factor:");
+				Log.log(Log.ERROR,this,e);
+				return null;
+			}
+		}
 	} //}}}
 
 	//{{{ getVocabularies() method
-	public Cons getVocabularies()
+	public synchronized Cons getVocabularies()
 	{
 		Cons vocabs = super.getVocabularies();
 
 		try
+		{
+			if(!closed)
 			{
 				Cons moreVocabs = (Cons)parseObject(eval("vocabs.")).car;
 				while(moreVocabs != null)
@@ -131,10 +169,12 @@ public class ExternalFactor extends DefaultVocabularyLookup
 					moreVocabs = moreVocabs.next();
 				}
 			}
+		}
 		catch(Exception e)
 		{
 			Log.log(Log.ERROR,this,e);
 		}
+
 		return vocabs;
 	} //}}}
 
@@ -142,13 +182,15 @@ public class ExternalFactor extends DefaultVocabularyLookup
 	/**
 	 * Search through the given vocabulary list for the given word.
 	 */
-	public FactorWord searchVocabulary(Cons vocabulary, String name)
+	public synchronized FactorWord searchVocabulary(Cons vocabulary, String name)
 	{
 		FactorWord w = super.searchVocabulary(vocabulary,name);
 		if(w != null)
 			return w;
 
 		try
+		{
+			if(!closed)
 			{
 				Cons result = parseObject(eval(FactorReader.unparseObject(name)
 					+ " "
@@ -164,19 +206,24 @@ public class ExternalFactor extends DefaultVocabularyLookup
 				w.stackEffect = (String)result.next().next().car;
 				return w;
 			}
+		}
 		catch(Exception e)
 		{
 			Log.log(Log.ERROR,this,e);
-			return null;
 		}
+
+		return new FactorWord("unknown",name);
 	} //}}}
 
 	//{{{ getCompletions() method
-	public void getCompletions(String vocab, String word, Set completions,
+	public synchronized void getCompletions(String vocab, String word, Set completions,
 		boolean anywhere)
 	{
 		super.getCompletions(vocab,word,completions,anywhere);
 
+		if(closed)
+			return;
+
 		try
 		{
 			/* We can't send words across the socket at this point in
@@ -212,6 +259,11 @@ public class ExternalFactor extends DefaultVocabularyLookup
 	 */
 	public synchronized void close()
 	{
+		if(closed)
+			return;
+
+		closed = true;
+
 		try
 		{
 			/* don't care about response */
@@ -225,6 +277,7 @@ public class ExternalFactor extends DefaultVocabularyLookup
 		
 		try
 		{
+			proc.waitFor();
 			in.close();
 			out.close();
 		}
@@ -233,9 +286,22 @@ public class ExternalFactor extends DefaultVocabularyLookup
 			// We don't care...
 			Log.log(Log.DEBUG,this,e);
 		}
+
+		proc = null;
+		in = null;
+		out = null;
+	} //}}}
+
+	//{{{ isClosed() method
+	public boolean isClosed()
+	{
+		return closed;
 	} //}}}
 
 	//{{{ Private members
+	private boolean closed;
+
+	private Process proc;
 	private DataInputStream in;
 	private DataOutputStream out;
 	

factor/FactorScanner.java

@@ -65,6 +65,11 @@ public class FactorScanner
 	 */
 	private int position = 0;
 
+	/**
+	 * Position of last word read.
+	 */
+	private int lastPosition = 0;
+
 	private ReadTable readtable;
 
 	/**
@@ -93,6 +98,12 @@ public class FactorScanner
 		this.readtable = readtable;
 	} //}}}
 
+	//{{{ getLine() method
+	public String getLine()
+	{
+		return line;
+	} //}}}
+
 	//{{{ getLineNumber() method
 	public int getLineNumber()
 	{
@@ -105,6 +116,12 @@ public class FactorScanner
 		return position;
 	} //}}}
 
+	//{{{ getLastColumnNumber() method
+	public int getLastColumnNumber()
+	{
+		return lastPosition;
+	} //}}}
+
 	//{{{ getFileName() method
 	public String getFileName()
 	{
@@ -117,6 +134,7 @@ public class FactorScanner
 		lineNo++;
 		line = in.readLine();
 		position = 0;
+		lastPosition = 0;
 		if(line != null && line.length() == 0)
 			nextLine();
 	} //}}}
@@ -178,6 +196,8 @@ public class FactorScanner
 		if(position == line.length())
 			return EOL;
 
+		lastPosition = position;
+
 		for(;;)
 		{
 			if(position >= line.length())
@@ -201,6 +221,7 @@ public class FactorScanner
 			case ReadTable.WHITESPACE:
 				if(buf.length() != 0)
 					return word(readNumbers,base);
+				lastPosition = position;
 				break;
 			case ReadTable.DISPATCH:
 				// note that s" is read as the word s", no

factor/FactorWord.java

@@ -29,11 +29,6 @@
 
 package factor;
 
-import java.util.*;
-
-/**
- * An internalized symbol.
- */
 public class FactorWord implements FactorExternalizable
 {
 	public String vocabulary;
@@ -47,7 +42,7 @@ public class FactorWord implements FactorExternalizable
 	public FactorParsingDefinition parsing;
 
 	/**
-	 * Stub for interpreter definitin.
+	 * Stub for interpreter definition.
 	 */
 	public FactorWordDefinition def;
 

factor/jedit/FactorPlugin.java

@@ -35,6 +35,7 @@ import java.util.*;
 import org.gjt.sp.jedit.gui.*;
 import org.gjt.sp.jedit.textarea.*;
 import org.gjt.sp.jedit.*;
+import org.gjt.sp.util.Log;
 import console.*;
 import sidekick.*;
 
@@ -80,9 +81,14 @@ public class FactorPlugin extends EditPlugin
 	 * It will start the interpreter if it's not already running.
 	 */
 	public synchronized static ExternalFactor getExternalInstance()
-		throws IOException, UnsupportedEncodingException
 	{
 		if(external == null)
+		{
+			Process p = null;
+			InputStream in = null;
+			OutputStream out = null;
+
+			try
 			{
 				String[] args = jEdit.getProperty("factor.external.args","-jedit")
 					.split(" ");
@@ -91,25 +97,38 @@ public class FactorPlugin extends EditPlugin
 				nargs[1] = jEdit.getProperty("factor.external.image");
 				nargs[2] = "-no-ansi";
 				System.arraycopy(args,0,nargs,3,args.length);
-			Process p = Runtime.getRuntime().exec(nargs);
+				p = Runtime.getRuntime().exec(nargs);
 				p.getErrorStream().close();
 
-			external = new ExternalFactor(
+				in = p.getInputStream();
-				p.getInputStream(),
+				out = p.getOutputStream();
-				p.getOutputStream());
+			}
+			catch(IOException io)
+			{
+				Log.log(Log.ERROR,FactorPlugin.class,
+					"Cannot start external Factor:");
+				Log.log(Log.ERROR,FactorPlugin.class,io);
+			}
+
+			external = new ExternalFactor(p,in,out);
 		}
 
 		return external;
 	} //}}}
 
+	//{{{ getFactorShell() method
+	public static FactorShell getFactorShell()
+	{
+		return ((FactorShell)ServiceManager.getService("console.Shell","Factor"));
+	} //}}}
+
 	//{{{ stopExternalInstance() method
 	/**
-	 * Stops the external interpreter. It will probably be restarted soon after.
+	 * Stops the external interpreter.
 	 */
 	public static void stopExternalInstance()
 	{
-		((FactorShell)ServiceManager.getService("console.Shell","Factor"))
+		getFactorShell().closeStreams();
-			.closeStreams();
 
 		if(external != null)
 		{
@@ -118,6 +137,17 @@ public class FactorPlugin extends EditPlugin
 		}
 	} //}}}
 	
+	//{{{ restartExternalInstance() method
+	/**
+	 * Restart the external interpreter.
+	 */
+	public static void restartExternalInstance()
+	{
+		stopExternalInstance();
+		getExternalInstance();
+		FactorPlugin.getFactorShell().openStreams();
+	} //}}}
+
 	//{{{ getSideKickParser() method
 	public static FactorSideKickParser getSideKickParser()
 	{

factor/jedit/FactorPlugin.props

@@ -8,8 +8,8 @@ plugin.factor.jedit.FactorPlugin.docs=/doc/jedit/index.html
 
 plugin.factor.jedit.FactorPlugin.depend.0=jedit 04.02.99.00
 plugin.factor.jedit.FactorPlugin.depend.1=plugin errorlist.ErrorListPlugin 1.3.2
-plugin.factor.jedit.FactorPlugin.depend.2=plugin sidekick.SideKickPlugin 0.3.1
+plugin.factor.jedit.FactorPlugin.depend.2=plugin sidekick.SideKickPlugin 0.3.2
-plugin.factor.jedit.FactorPlugin.depend.3=plugin console.ConsolePlugin 4.0.1
+plugin.factor.jedit.FactorPlugin.depend.3=plugin console.ConsolePlugin 4.0.2
 
 #! Menu
 plugin.factor.jedit.FactorPlugin.menu=factor-listener \
@@ -72,6 +72,8 @@ factor.extract-word-where.message=This command can only be used inside a word de
 factor.shell.opening=Opening listener connection
 factor.shell.closing=Closing listener connection
 factor.shell.not-waiting=Not accepting input at this time
+factor.shell.no-connection=Could not establish connection with external Factor.\n\
+	Check settings.
 
 # Option pane
 plugin.factor.jedit.FactorPlugin.option-pane=factor

factor/jedit/FactorShell.java

@@ -78,13 +78,17 @@ public class FactorShell extends Shell
 	 */
 	public void printPrompt(Console console, Output output)
 	{
+		ConsoleState state = null;
 		try
 		{
-			getConsoleState(console).packetLoop(output);
+			state = getConsoleState(console);
+			state.packetLoop(output);
 		}
 		catch(Exception e)
 		{
 			output.print(console.getErrorColor(),e.toString());
+			if(state != null)
+				state.closeStream();
 			Log.log(Log.ERROR,this,e);
 		}
 	} //}}}
@@ -102,13 +106,17 @@ public class FactorShell extends Shell
 	public void execute(Console console, String input,
 		Output output, Output error, String command)
 	{
+		ConsoleState state = null;
 		try
 		{
-			getConsoleState(console).readResponse(output,command);
+			state = getConsoleState(console);
+			state.readResponse(command,output);
 		}
 		catch(Exception e)
 		{
 			output.print(console.getErrorColor(),e.toString());
+			if(state != null)
+				state.closeStream();
 			Log.log(Log.ERROR,this,e);
 		}
 		finally
@@ -126,6 +134,20 @@ public class FactorShell extends Shell
 	{
 	} //}}}
 
+	//{{{ openStreams() method
+	/**
+	 * Open all listener connections. Should be called after Factor is restarted.
+	 */
+	public void openStreams()
+	{
+		Iterator iter = consoles.values().iterator();
+		while(iter.hasNext())
+		{
+			ConsoleState state = (ConsoleState)iter.next();
+			state.openStream();
+		}
+	} //}}}
+
 	//{{{ closeStreams() method
 	/**
 	 * Close all listener connections. Should be called before Factor is restarted.
@@ -162,21 +184,40 @@ public class FactorShell extends Shell
 	class ConsoleState
 	{
 		private Console console;
+		private Output output;
 		private FactorStream stream;
 		private boolean waitingForInput;
 		
 		ConsoleState(Console console)
 		{
 			this.console = console;
+			this.output = console.getShellState(FactorShell.this);
 		}
 
-		void openStream(Output output) throws Exception
+		void openStream()
 		{
+			if(stream != null)
+				return;
+
+			output.print(console.getInfoColor(),
+				jEdit.getProperty("factor.shell.opening"));
+
+			stream = FactorPlugin.getExternalInstance().openStream();
 			if(stream == null)
 			{
 				output.print(console.getInfoColor(),
-					jEdit.getProperty("factor.shell.opening"));
+					jEdit.getProperty("factor.shell.no-connection"));
-				stream = FactorPlugin.getExternalInstance().openStream();
+			}
+			else
+			{
+				try
+				{
+					packetLoop(output);
+				}
+				catch(Exception e)
+				{
+					Log.log(Log.ERROR,this,e);
+				}
 			}
 		}
 
@@ -187,7 +228,7 @@ public class FactorShell extends Shell
 				if(stream != null)
 				{
 					waitingForInput = false;
-					console.print(console.getInfoColor(),
+					output.print(console.getInfoColor(),
 						jEdit.getProperty("factor.shell.closing"));
 					stream.close();
 				}
@@ -218,7 +259,10 @@ public class FactorShell extends Shell
 			if(waitingForInput)
 				return;
 
-			openStream(output);
+			openStream();
+
+			if(stream == null)
+				return;
 
 			for(;;)
 			{
@@ -239,11 +283,14 @@ public class FactorShell extends Shell
 			}
 		}
 
-		void readResponse(Output output, String command) throws Exception
+		void readResponse(String command, Output output) throws Exception
 		{
 			if(waitingForInput)
 			{
-				openStream(output);
+				openStream();
+
+				if(stream == null)
+					return;
 
 				stream.readResponse(command);
 				waitingForInput = false;
@@ -251,7 +298,7 @@ public class FactorShell extends Shell
 			}
 			else
 			{
-				console.print(console.getErrorColor(),
+				output.print(console.getErrorColor(),
 					jEdit.getProperty("factor.shell.not-waiting"));
 			}
 		}

factor/jedit/RestartableFactorScanner.java

@@ -48,8 +48,16 @@ public class RestartableFactorScanner extends FactorScanner
 	//{{{ error() method
 	public void error(String msg) throws FactorParseException
 	{
+		String line = getLine();
+		int col = getColumnNumber();
+		if(getReadTable().getCharacterType(line.charAt(col - 1))
+			== ReadTable.WHITESPACE)
+		{
+			col--;
+		}
+
 		errors.addError(ErrorSource.ERROR,getFileName(),
 			/* Factor line #'s are 1-indexed */
-			getLineNumber() - 1,0,0,msg);
+			getLineNumber() - 1,getLastColumnNumber(),col,msg);
 	} //}}}
 }

factor/jedit/WordPreview.java

@@ -76,6 +76,9 @@ public class WordPreview implements ActionListener, CaretListener
 	{
 		try
 		{
+			if(FactorPlugin.getExternalInstance().isClosed())
+				return;
+
 			showPreview();
 		}
 		catch(IOException e)

library/vectors.factor

@@ -70,10 +70,6 @@ DEFER: vector-map
     #! Shallow copy of a vector.
     [ ] vector-map ;
 
-: ?vector= ( n vec vec -- ? )
-    #! Reached end?
-    drop vector-length number= ;
-
 : vector-length= ( vec vec -- ? )
     vector-length swap vector-length number= ;