opening-sophie-books-in-command-line - Provide the possibility to open Sophie books in command line.

[todor]

• All files supported by Sophie must have the possibility to be opened in command line by the author and reader applications.
• When multiple files are being opened, they must be loaded in one application with author and only the first argument for the reader.
• When the files are opened one by one they must be loaded in different applications.

[stefan]

Analysis is ok.

[stefan]

Changes:

• Main class - checking-asserting that arguments are >= 2, and passing the arguments to the Launcher.start(...) method:

  assert args.length >= 2;
  
  String configFileName = args[0];
  String edition = args[1];
  String[] files = new String[args.length - 2];
  	
  String bundlesListFileName = "/" + edition + "-run" + "/" + configFileName;
  
  for (int i = 0; i < args.length - 2; i++) {
  	files[i] = LocationPrefix.FILE + args[i + 2];
  }
  

• Launcher class - new system property is introduced and the last one (it could be used only or one book reference) is dismissed:

  System.getProperties().put("sophie2.loadOnStartUpFiles", bookRefs);
  

• AuthorModule class - now is checked for start up books:

  private ResourceRefR4[] checkStartUpBooks() throws IOException {
  	String[] startUpBookFiles = (String[]) System.getProperties().get("sophie2.loadOnStartUpFiles");
  	if (startUpBookFiles == null) {
  		return null;
  	}
  	
  	ResourceRefR4[] resultRefs = new ResourceRefR4[startUpBookFiles.length];
  	for (int i = 0; i < startUpBookFiles.length; i++) {
  		ResourceRefR4 bookRef = ResourceRefR4.make(startUpBookFiles[i]);
  		URI uri = bookRef.toUri();
  		if (!uri.isAbsolute()) {
  			SophieLog.warnf("Cannot open non absolute ref[%d]: %s.", i, bookRef);
  			resultRefs[i] = null;
  			continue;
  		}
  		
  		InputStream from = null;
  		FileOutputStream to = null;
  		
  		try {
  			from = uri.toURL().openStream();
  			File resourceFile = File.createTempFile("sophieStartupBook " + i, null);
  			resourceFile.deleteOnExit();
  			to = new FileOutputStream(resourceFile);
  			BinData.transport(from, to);
  			resultRefs[i] = ResourceRefR4.make(resourceFile);
  		} finally {
  			if (to != null) {
  				to.close();
  			}
  			if (from != null) {
  				from.close();
  			}
  		}
  	}
  	return resultRefs;
  }
  

• ReaderModule class - same as AuthorModule:

  private ResourceRefR4 checkStartupBook() throws IOException {
  		String[] startUpBookFiles = (String[]) System.getProperties().get("sophie2.loadOnStartUpFiles");
  	if (startUpBookFiles == null) {
  		return null;
  	}
  	//open only first book.
  	
  	ResourceRefR4 bookRef = ResourceRefR4.make(startUpBookFiles[0]);
  	URI uri = bookRef.toUri();
  ...
  

[meddle]

• Why you copy the books in the tmp directory before opening them. Such a thing is not wanted by the analysis... And don't forget that the books with linked resources will lose their links this way. The method that saves these links is ResourceFilesUtil#storeFile(ResourceH res, File dest). You can easily get the ResourceH from the ref...
  • But again why you need to do that copying in the temp???
• You didn't mention the applet in the design (it is in your patch though)
• The analysis is unclear (you marked it with 4, but see point 3). The third analysis point, I think is about the reader. That the reader must open all of the files but in different applications.
  • The second point and the third are confusing...
  • I talked with todor.
    • The third point means that if you open one file with sophie, the next time you open file by the command line, it will open in different Sophie 2 application, not the already running one.
• So the design is not by the analysis (The wrongly copied resources in the tmp, may be you saw tsachev's code for the applet... There the files must be copied somewhere I guess... and the code is old so, doesn't save the linked resources)
• Note that when the Applet begin working it should use your new logic.

2.5p

[stefan]

Replying to meddle:

• Why you copy the books in the tmp directory before opening them. Such a thing is not wanted by the analysis... And don't forget that the books with linked resources will lose their links this way. The method that saves these links is ResourceFilesUtil#storeFile(ResourceH res, File dest). You can easily get the ResourceH from the ref...
  • But again why you need to do that copying in the temp???
• You didn't mention the applet in the design (it is in your patch though)
• The analysis is unclear (you marked it with 4, but see point 3). The third analysis point, I think is about the reader. That the reader must open all of the files but in different applications.
  • The second point and the third are confusing...
  • I talked with todor.
    • The third point means that if you open one file with sophie, the next time you open file by the command line, it will open in different Sophie 2 application, not the already running one.
• So the design is not by the analysis (The wrongly copied resources in the tmp, may be you saw tsachev's code for the applet... There the files must be copied somewhere I guess... and the code is old so, doesn't save the linked resources)
• Note that when the Applet begin working it should use your new logic.

2.5p

regarding 3rd point of the design review:

• For me, 3rd point of the analysis is very clear. It requires that every time a book is opened (different commands), a new application should be started:

When the files are opened one by one they must be loaded in different applications. - as a opposite example I would give you photoshop, where, when file is opened, if there is an active photoshop application running, the file is opened in it. It was specifically discussed and came to the conclusion that is needed that kind of behavior - opening different files in different applications.

[stefan]

• 1st point (of the review) is reviewed and fixed.
• 2nd one doesn't make sense - its same as the first one.
• 3rd point - It is very clear why it was not mentioned - its only wrapping the previous single argument and passing it as an array of refs. I think the code speaks of it self - it doesn't bear any logic.
• 4th point is answered in the previous reply.
• 5th point - I disagree.
• 6th - good that you have talked :).
• 7th is explained in the previous reply as was the 4th.
• 8th is fixed in this design (its much of the same, simply the copying is fixed).

[stefan]

a patch for the second attempt on design.

[meddle]

• Good...
  • The second point of my review was telling you how to save the linked resources when you copy resource file somewhere, I think it is important for you to know that.
  • The third was a question 'why on the first place you are copying them'... So they are different... In the second there is question two, but for me it was very important to understand why, may be you had some special intention here...
• The analysis was not clear for me...
• Describe all the changes in the design, the code at the design phase is important and helps, but the design is the documentation of your task.

4p

[stefan]

Only differences of the last design is that no longer files are copied in temp folder:

• AuthorModule :

  private ResourceRefR4[] checkStartUpBooks() {
  		String[] startUpBookFiles = (String[]) System.getProperties().get("sophie2.loadOnStartUpFiles");
  		if (startUpBookFiles == null) {
  			return null;
  		}
  		
  		ResourceRefR4[] resultRefs = new ResourceRefR4[startUpBookFiles.length];
  		for (int i = 0; i < startUpBookFiles.length; i++) {
  			ResourceRefR4 bookRef;
  			if (startUpBookFiles[i].startsWith(LocationPrefix.FILE)) {
  				bookRef = ResourceRefR4.make(startUpBookFiles[i]);
  			} else {
  				bookRef = ResourceRefR4.make(LocationPrefix.FILE + startUpBookFiles[i]);
  			}
  			URI uri = bookRef.toUri();
  			if (!uri.isAbsolute()) {
  				SophieLog.warnf("Cannot open non absolute ref[%d]: %s.", i, bookRef);
  				resultRefs[i] = null;
  				continue;
  			}
  			resultRefs[i] = bookRef;
  		}
  		return resultRefs;
  	}
  

• ReaderModule

  	private ResourceRefR4 checkStartupBook(){
  		String[] startUpBookFiles = (String[]) System.getProperties().get("sophie2.loadOnStartUpFiles");
  		if (startUpBookFiles == null) {
  			return null;
  		}
  		//open only first book.
  		ResourceRefR4 bookRef;
  		if (startUpBookFiles[0].startsWith(LocationPrefix.FILE)) {
  			bookRef = ResourceRefR4.make(startUpBookFiles[0]);
  		} else {
  			bookRef = ResourceRefR4.make(LocationPrefix.FILE + startUpBookFiles[0]);
  		}
  		URI uri = bookRef.toUri();
  		
  		if (!uri.isAbsolute()) {
  			SophieLog.warnf("Cannot open non absolute ref %s.", bookRef);
  			return null;
  		}
  
  		return bookRef;
  	}
  

[stefan]

implementation code - 2396_2.patch.

[meddle]

Merged into the trunk at [9029].

• We had many problems trying to run it on different platforms than linux.
• Write your name as an author of the Author and Reader modules...

3p

[meddle]

Closing all the tickets before M Y1