CatCommand doesn't handle filename patterns

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

The 'cat' command doesn't handle filename patterns (wildcards) when called via the default or redirecting command interpreters. This is because it gets its arguments using getValues() rather than getFiles().

This patch corrects this, and incorporates some minor code simplification and error message tweaks.

AttachmentSize
issues_953.71 KB

#1

Status:active» closed

With this patch URLs are not working any more.
Should we make an effort to support URLs as arguments for cat?
It's been a handy feature so far.

#2

I'll look into this. It may just be a silly bug. But there is also a problem at the Syntax level.

The root problem here is that the Syntax for 'cat' does not distinguish between the cases normal pathnames and URLs. In the former case you want wildcard expansion, in the latter you probably don't. (If you do want wildcard expansion for URLs, it needs a different implementation to the filename case). By declaring cat's argument as a FileArgument, we've created a Syntax that doesn't match the underlying semantics of what 'cat' is going to do. (This also would apply to argument completion ... if we decide to make completion of a FileArgument do "helpful" things with wildcards.)

Here are some solutions:

  • Create a new command (e.g. called 'wget') for fetching files via HTTP. This is my preference. In the future, it will allow us to add parameters that are specific to the HTTP use case.
  • Give 'cat' an alternative syntax for dealing with the "wget" case; e.g. "cat [ ...]" versus "cat -u ...". This works, but it is arguably a case of "creeping featurism" (I'll explain that somewhere else).
  • Try to come up with a new Argument subtype that supports both files and URLs, and does the appropriate kind of wildcard expansion. (Sounds tricky, and presents a big problem for the Bjorne use-case.)

#3

The third option (special argument for files and URLs) came to my mind as a solution too for handling the current case.
Also the copy command has this property, that the source can be an URL. (On the other hand the classpath and plugin command accept only URLs while simple local paths would make sense too.)
Obviously we need normal code completion and wildcards as long as the argument is a file and probably an URL too with the file protocol.
Beyond these two commands a general design idea is taking shape where certain commands where it makes sense will be enhanced with network friendly features.
While it's important for the bjorne shell to be compatible with the unix version, we should also pay attention to the fact that with the default shell we are creating a new shell where there should be place for innovation, experimentation and the two shells possibly should not interfer in this sense.

#4

I've figured out what the bug is. You'll love this!

Suppose that you run "cat ftp://ftp.gnu.org/README". Eventually, CatCommand calls FileArgument.getFiles(), and this is what happens:

  1. It examines the argument value string and determines it doesn't require wildcard expansion.
  2. It takes the argument value string and turns it into a File.
  3. It returns a one element File array to the caller.
  4. The CatCommand.execute method extracts the zero'th File.
  5. It turns the back into a String, then uses the string to construct a URL.
  6. It calls url.openStream() ... which fails with a connection timeout.

The clue is that the connection timeout message says that the URL it was trying to connect to is "ftp:/ftp.gnu.org/README". Note that we've lost a "/"!!

I'm pretty sure that what has happened is that the File constructor is quietly turning the "//" into a "/". That's OK for a pathname, but it changes the meaning of a url such that openStream() is tries to connect to an FTP server at this machine's IP address.

#5

Here's a new version of the patch. This version adds an alternative syntax for 'cat' for the URL case:

cat -u <URL> ...
cat [ <FileName> ... ]

This patch depends on the patch to Syntax that allows it to recognize named Parameters to make the "cat -u" syntax work.

#6

This is OK. Committed.

#7