Tidyup patch for Command/CommandLine and classes that use it

Project:JNode Core
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed
Description

This patch changes some core APIs as follows:

  • A CommandLine object is no longer an Iterator<String>. Instead, it implements Iterable<String> so that a client don't have to worry about reentrancy when iterating
    the arguments. In addition, there is a new method for iterating the argumens as Tokens, and new constructors for building a CommandLine from command and argument Tokens.

  • The (optional) AbstractCommand is an abstract base class for classes that implement the Command interface. This provides an execute(String[]) method that simplifies the stereotypical implementation of the "maim" entry point method, and an exit(int) method.

The patch changes a large number of command classes to use the modified APIs; e.g. to extend AbstractCommand, and call the exit method to indicate command failure.

The patch changes the way that the interpreters handle redirection and pipe streams. (The interpreters now use StreamMarker objects to denote certain distinguished streams. These are mapped to the corresponding real stream by the invokers. This makes it easier to get the stream opening / closing logic correct ... especially in bjorne.)

The patch changes the way that command return codes (e.g. from the "exit" method) are handled internally by the CommandShell infrastructure. This relates to a bug; see below.

Finally, it incorporate fixes (and non-fixes) for a couple of problems that Levente spotted.

  • There is a tweak in to ConsoleCommand that will warn the user that System.in,out,err will NOT work as expected from a "console -n" shell if you are using the "default" or "thread" invoker. This tweak is a short term hack. IMO, the long term solution is to get rid of these invokers entirely ... but that's a different topic. (The proclet mechanism used by "proclet" invoker makes these streams almost work as expected ... and an invoker based on isolates would fully address the problem.)
  • There is a fix for a bug that stopped "internal" commands from running. The way that I was handling return codes was broken for "internal" commands.
AttachmentSize
issues_90162.44 KB

#1

Status:active» closed

P.S. The patch also includes a new UnixTestCommand class. This is intended to be an analogue for the UNIX 'test' command. Unfortunately, a lot of the standard file predicates are not implemented yet.

#2

I've committed this one though not sure if we should pay that much attention to fixing various minor coding style issues at this point. I hope things like jackpot (part of netbeans) will be of great help in being more effective with such things in the future.

#3

Thanks for committing the patch.

On your point about spending time to fix coding style issues:

For purely internal examples of poor style, I agree with you. If the code works well enough, and "application" programmers don't see it, we can put off tidying it up any poor style indefinitely.

However, when the bad style results in broken behavior or messy / hard to use public APIs, stylistic tidyups (and adding javadocs) can be important. This is particularly so for widely used APIs like Command and CommandLine:

  • New JNode users tend to first look at the APIs they need to use to interface their apps with JNode. If the APIs are a mess, they get a bad impression.
  • When JNode takes off, we will soon reach a point where some JNode compatible apps don't live in the JNode source repository. When that happens, it will be much harder to fix broken APIs that these apps depend on. Various problems in Sun's JDK 1.0 APIs persist to this day for this very reason; e.g. Thread.stop(), System.arrayCopy(), issues with InputStream / OutputStream / PrintStream.

#4

That is correct!

#5