reworked external editor feature...

Signed-off-by: Alex A. Naanou <alex.nanou@gmail.com>

ui (gen4)/features/external-editor.js

@@ -4,6 +4,10 @@
 *
 **********************************************************************/
 
+if(typeof(process) != 'understand'){
+	var pathlib = require('path')
+}
+
 define(function(require){ var module = {}
 
 //var DEBUG = DEBUG != null ? DEBUG : true
@@ -41,7 +45,7 @@ var ExternalEditorActions = actions.Actions({
 
 		// XXX should this be a dict???
 		// 		...a list is simpler for sorting...
-		'_external-editors': [
+		'external-editors': [
 			{
 				// NOTE: empty means use app name...
 				title: 'System default',
@@ -51,23 +55,17 @@ var ExternalEditorActions = actions.Actions({
 				arguments: '',
 				target: '',
 			},
+			/*
 			{
 				title: 'IrfanView',
 				path: 'C:/Program Files (x86)/IrfanView/i_view32.exe',
 				arguments: '',
 				target: '',
 			},
+			*/
 		],
 
-		'external-editors': [
+		// XXX this is not used yet...
-			// XXX system default might be different on different systems...
-			['System default|"$PATH"'],
-
-			// XXX for some reason irfanview doesnot open a path passed 
-			// 		as argument unless it uses only '\' and not '/'
-			['IrfanView|"C:/Program Files (x86)/IrfanView/i_view32.exe" "$PATH"'],
-		],
-
 		'external-editor-targets': [
 			'Best preview',
 			//'Original image',
@@ -78,14 +76,28 @@ var ExternalEditorActions = actions.Actions({
 	// XXX this still needs image type and better support for OS paths 
 	// 		...irfanview for instance does not understand '/' in paths 
 	// 		while windows in general have no problem...
+	// XXX target is not yet used...
 	openInExtenalEditor: ['Edit/Open with external editor',
 		function(editor, image, type){
-			editor = typeof(editor) == typeof('str') ? editor 
+			editor = editor || 0
-				: this.config['external-editors'][editor == null ? 0 : editor]
+			editor = typeof(editor) == typeof('str') ?
-			editor = editor ? editor[0] : '$PATH'
+					this.config['external-editors']
+						// XXX should we use other criteria???
+						.filter(function(e){ return editor == e.title })[0]
+				: this.config['external-editors'][editor]
+
+			if(editor == null){
+				// XXX ERR???
+				return
+			}
+
+			// XXX
+			//var target = editor.target == '' ? 'Best preview' : editor.target
 
 			// get the path part...
-			editor = editor.split(/\|/g).pop()
+			//editor = editor.split(/\|/g).pop()
+			editor = (editor.path != '' ? '"'+ editor.path +'" ' : '')
+				+ ((editor.arguments == '' || !editor.arguments) ? '"$PATH"' : editor.arguments)
 
 			// get the image...
 			var img = this.images[this.data.getImage(image)]
@@ -146,8 +158,17 @@ var ExternalEditorUIActions = actions.Actions({
 		function(editor){
 			var that = this
 
+			editor = editor || 0
+			editor = typeof(editor) == typeof('str') ?
+					this.config['external-editors']
+						// XXX should we use other criteria???
+						.filter(function(e){ 
+							return editor == e.title 
+								|| editor == pathlib.basename(e.path) })[0]
+				: this.config['external-editors'][editor]
+
 			// XXX STUB: get the real editor...
-			var editor = {
+			editor = editor || {
 				// NOTE: empty means use app name...
 				title: '',
 				// NOTE: empty means system to select editor...
@@ -157,6 +178,8 @@ var ExternalEditorUIActions = actions.Actions({
 				target: '',
 			}
 
+			var editor_i = this.config['external-editors'].indexOf(editor)
+
 			var o = overlay.Overlay(this.ribbons.viewer, 
 				browse.makeLister(null, function(_, make){
 					make(['Title: ', function(){ return editor.title || '' }])
@@ -231,8 +254,16 @@ var ExternalEditorUIActions = actions.Actions({
 
 					make(['Save'])
 						.on('open', function(){
-							// XXX save stuff...
+							var editors = that.config['external-editors']
-							// XXX
+
+							// updated editor...
+							if(editor_i >= 0){
+								that.config['external-editors'] = editors.slice()
+
+							// new editor...
+							} else {
+								that.config['external-editors'] = editors.concat([editor])
+							}
 
 							o.close()
 						})
@@ -248,56 +279,13 @@ var ExternalEditorUIActions = actions.Actions({
 	listExtenalEditors: ['Edit/List external editors',
 		function(){
 			var that = this
-
-			// build the list...
-			var list = {}
-			var editors = this.config['external-editors'].slice()
-			editors
-				.forEach(function(e, i){
-					list[e[0].split(/\|/g)[0]] = function(){
-						that.openInExtenalEditor(i)
-					}
-				})
-
 			var closingPrevented = false
-
+			var editors = this.config['external-editors'] || []
-			// XXX STUB: use a top button...
-			// XXX update the list...
-			list['Add new editor...'] = function(){
-				closingPrevented = true
-				// XXX open 'new editor...' dialog...
-				var b = overlay.Overlay(that.ribbons.viewer, 
-					browseWalk.makeWalk(
-							null, '/', 
-							// XXX
-							'*+(exe|cmd|ps1|sh)',
-							{})
-						// path selected...
-						.open(function(evt, path){ 
-							// XXX
-							//this.parent.close()
-
-							// add a pretty name...
-							editors.push([path+'|"'+ path +'" "$PATH"'])
-							that.config['external-editors'] = editors
-
-							// XXX update the editor list...
-
-							// is this the correct way to do this???
-							b.close()
-							o.close()
-							that.listExtenalEditors()
-						}))
-						.close(function(){
-							o.focus()
-						})
-				return b
-			}
 
 			// element index...
 			var _getEditor = function(str){
 				return editors
-					.map(function(e){ return e[0].split(/\|/g)[0] })
+					.map(function(e){ return e.title || pathlib.basename(e.path) })
 					.indexOf(str)
 			}
 
@@ -305,9 +293,62 @@ var ExternalEditorUIActions = actions.Actions({
 
 			// build the dialog...
 			var o = overlay.Overlay(this.ribbons.viewer, 
-				browse.makeList(null, list, {
+				//browse.makeList(null, list, {
+				browse.makeLister(null, 
+					function(_, make){
+						editors
+							.forEach(function(e, i){
+								make([function(){ return e.title || pathlib.basename(e.path)}])
+									.on('open', function(){
+										that.openInExtenalEditor(i)
+									})
+							})
+
+						make(['Add new editor...'])
+							.on('open', function(){
+								closingPrevented = true
+								// XXX open 'new editor...' dialog...
+								var b = overlay.Overlay(that.ribbons.viewer, 
+									browseWalk.makeWalk(
+											null, '/', 
+											// XXX
+											'*+(exe|cmd|ps1|sh)',
+											{})
+										// path selected...
+										.open(function(evt, path){ 
+											// XXX
+											//this.parent.close()
+
+											// add a pretty name...
+											editors.push({
+												path: path,
+											})
+											that.config['external-editors'] = editors
+
+											// XXX update the editor list...
+
+											// is this the correct way to do this???
+											b.close()
+											o.close()
+											that.listExtenalEditors()
+										}))
+										.close(function(){
+											o.focus()
+										})
+								return b
+							})
+					}, 
+					{
 						// add item buttons...
 						itemButtons: [
+							// edit...
+							['edit', 
+								function(p){
+									that.externalEditorDialog(p)
+										.close(function(){
+											o.client.update()
+										})
+								}],
 							// move to top...
 							['♦', 
 								function(p){

ui (gen4)/features/filesystem.js

@@ -767,7 +767,7 @@ var FileSystemWriterActions = actions.Actions({
 	
 	// NOTE: with no arguments this will save index to .location.path
 	// XXX should this return a promise??? ...a clean promise???
-	saveIndex: ['- File/Save index',
+	saveIndex: ['- File/Save',
 		function(path, logger){
 			var that = this
 			path = path || this.location.path
@@ -797,6 +797,29 @@ var FileSystemWriterActions = actions.Actions({
 					that.location.method = 'loadIndex'
 				})
 		}],
+	// XXX should this be a UI action???
+	// 		...at this point this depends on .saveIndexHere(..), thus 
+	// 		it is here...
+	// XXX should this return a promise???
+	saveFullIndex: ['File/Save (full)',
+		function(){
+			return this
+				.markChanged('all')
+				.saveIndexHere()}],
+
+
+	// XXX ways to treat a collection:
+	// 		- crop data
+	// 		- independent index
+	// XXX save to: .ImageGrid/collections/<title>/
+	// XXX move to a feature???
+	// XXX API: save/load/list/remove
+	// 		...need to track save location (not the save as the index)...
+	// XXX
+	saveCollection: ['- File/Save collection',
+		function(title){
+			// XXX
+		}],
 
 	// Export current state as a full loadable index
 	//
@@ -806,7 +829,7 @@ var FileSystemWriterActions = actions.Actions({
 	// XXX add preview selection...
 	// XXX handle .image.path and other stack files...
 	// XXX local collections???
-	exportIndex: ['- File/Export index',
+	exportIndex: ['- File/Export/Export index',
 		function(path, logger){
 			logger = logger || this.logger
 
@@ -914,12 +937,13 @@ var FileSystemWriterActions = actions.Actions({
 		}],
 	
 	// XXX might also be good to save/load the export options to .ImageGrid-export.json
-	// XXX resolve env variables in path...
+	// XXX resolve env variables in path... (???)
-	// XXX make custom previews...
+	// XXX make custom previews (option)...
 	// 		...should this be a function of .images.getBestPreview(..)???
 	// XXX report errors...
 	// XXX stop the process on errors...
-	exportDirs: ['File/Export as nested directories',
+	// XXX use tasks...
+	exportDirs: ['- File/Export/Export ribbons as directories',
 		function(path, pattern, level_dir, size, logger){
 			logger = logger || this.logger
 			var that = this
@@ -1178,37 +1202,28 @@ var FileSystemWriterUIActions = actions.Actions({
 
 	// XXX this needs feedback...
 	// XXX should this return a promise???
-	saveIndexHere: ['File/Save',
+	saveIndexHere: ['- File/',
 		function(){ 
 			if(this.location.path){ 
-				this.saveIndex(this.location.path) 
+				this.saveIndex() 
 
 			} else {
 				this.browseSaveIndex()
 			}
 		}],
-	// XXX should this be a UI action???
-	// 		...at this point this depends on .saveIndexHere(..), thus 
-	// 		it is here...
-	// XXX should this return a promise???
-	saveFullIndex: ['File/Save full',
-		function(){
-			return this
-				.markChanged('all')
-				.saveIndexHere()}],
 
 	// XXX add ability to create dirs...
 	// XXX this needs feedback...
 	// XXX should this return a promise???
-	browseSaveIndex: ['File/Save index to...', 
+	browseSaveIndex: ['- File/Save index to...', 
 		makeBrowseProxy('saveIndex', function(){
 			this.location.method = 'loadIndex' })],
 	// XXX need to be able to make dirs...
-	browseExportIndex: ['File/Export/Index to...',
+	browseExportIndex: ['File/Export/Export Index to...',
 		makeBrowseProxy('exportIndex')],
 	// XXX need to be able to make dirs...
 	// XXX STUB
-	browseExportDirs: ['File/Export/Images to...',
+	browseExportDirs: ['File/Export/Export Images to...',
 		makeBrowseProxy('exportDirs')],
 
 

ui (gen4)/lib/util.js

@@ -273,6 +273,7 @@ if(typeof(jQuery) != typeof(undefined)){
 			.prop('contenteditable', true)
 			// make the element focusable and selectable...
 			.attr('tabindex', '0')
+			.addClass('editable-field')
 			// NOTE: this will also focus the element...
 			.selectText()
 			.keydown(function(){ 

ui (gen4)/lib/widget/browse.js

@@ -2223,7 +2223,7 @@ var makeLister =
 module.makeLister = function(elem, lister, options){
 	var opts = {}
 	for(var k in options){
-		opts[k] = rest[k]
+		opts[k] = options[k]
 	}
 	opts.list = lister
 	return Lister(elem, opts)