tee command implementation

Project:JNode Shell
Component:Code
Category:task
Priority:normal
Assigned:zephyr
Status:fixed
Description

This is a implementation of the tee command based on the description found at: http://www.opengroup.org/onlinepubs/9699919799/utilities/tee.html

The append switch works on the blackbox tests but doesn't work when using JNode on a virtual machine.

AttachmentSize
tee-command-patch.txt7.74 KB

#1

Please give details about the failure under jnode so that one (especially an experimented jnode developer) could have a chance to help you without having to apply, build and test your patch ?
Thanks.

#2

The problem is that there is no failure, the file simply doesn't get appended. The file contents are the same as before the command.

#3

There is already an existing issue for File append not working on JNode. If I remember correctly, support for opening files for appending is not present at the FS independent level.

#4

2 things :
- you are not closing the streams : I think it works ouside of jnode because, when the jvm is shut down, the streams are closed automatically => it's a good practice to close all the stream you've opened.
- could you give the patch against /jnode/cli instead of cli (I assume your patch is against cli but it's better to avoid having to guess that) ?
thanks

#5

Here is the new patch. I tried to generate it against /jnode/cli, hope it's ok.

Only difference from previous is the closing of the streams in the end.

AttachmentSize
tee-command-patch_v1.txt8.08 KB

#6

ok, it's working now but before I commit your work, you should read and apply that : http://www.jnode.org/node/2662.
Examples :

  • "private static final String help_file" should be "private static final String HELP_FILE"
  • some javadoc is missing

There is also an issue that could be revealed in some exceptional cases : if an error (IOException or anything else) happen after the first IOUtils.openOutputstream, the already opened streams won't be closed. The solution is to put all in a try { } finally { } block : in the finally, you will close all the opened streams

#7

something else : it's always a good idea to add a link in the javadoc when you are implementing a class from some reference document accessible online.
In your case, you should add a link to http://www.opengroup.org/onlinepubs/9699919799/utilities/tee.html.

#8

Here is a new patch with the following updates:
- Added the file header from the template
- Added a reference to the command documentation in the javadoc
- Added missing javadocs
- Re-factored some variable names
- Moved the closing of the files to a finally block
- Corrected the maximum number of files that could be used in the command
- Added backbox tests for the maximum number of files

AttachmentSize
tee-command-patch_v2.txt16.18 KB

#9

Thanks for your contribution !
I have committed your patch : it's under revision 5642.

#10

Status:patch (code needs review)» closed

#11

Status:closed» active

The task should not be closed because (from what you said) the tee command is not complete.
So, I think 'active' is the most appropriate here.

#12

I've closed the task because the command itself is complete, the problem is that the append functionality is not implemented in JNode as mentioned in #3.

#13

Status:active» fixed

Thanks for the hint, marking fixed.