A little better support for non-term-256 colors in web config

Fix for a deadlock when autoloading a function triggers autoloading another function

4 files changed, 63 insertions, 31 deletions

diff --git a/autoload.cpp b/autoload.cpp
index 57de29cc..2d1b8394 100644
--- a/autoload.cpp
+++ b/autoload.cpp
@@ -70,28 +70,20 @@ int autoload_t::load( const wcstring &cmd, bool reload )
 	CHECK_BLOCK( 0 );
     ASSERT_IS_MAIN_THREAD();
     
-	env_var_t path_var;
+	env_var_t path_var = env_get_string( env_var_name );
     
-    /* Do some work while locked, including determing the path variable */
+    /*
+      Do we know where to look?
+    */
+    if( path_var.empty() )
+        return 0;
+    
+    /* Check if the lookup path has changed. If so, drop all loaded files. path_var may only be inspected on the main thread. */
+    if( path_var != this->last_path )
     {
-        scoped_lock locker(lock); 
-        path_var = env_get_string( env_var_name );
-        
-        /*
-          Do we know where to look?
-        */
-        if( path_var.empty() )
-            return 0;
-        
-        /*
-          Check if the lookup path has changed. If so, drop all loaded
-          files.
-        */
-        if( path_var != this->path )
-        {
-            this->path = path_var;
-            this->evict_all_nodes();
-        }
+        this->last_path = path_var;
+        scoped_lock locker(lock);
+        this->evict_all_nodes();
     }
     
     /** Warn and fail on infinite recursion. It's OK to do this because this function is only called on the main thread. */
diff --git a/autoload.h b/autoload.h
index b94876a7..d22229ea 100644
--- a/autoload.h
+++ b/autoload.h
@@ -56,7 +56,7 @@ private:
     const size_t builtin_script_count;
 
     /** The path from which we most recently autoloaded */
-    wcstring path;
+    wcstring last_path;
 
 	/**
 	   A table containing all the files that are currently being
diff --git a/share/tools/web_config/index.html b/share/tools/web_config/index.html
index fcae12ec..ca7eaaeb 100644
--- a/share/tools/web_config/index.html
+++ b/share/tools/web_config/index.html
@@ -417,7 +417,6 @@ function switch_tab(new_tab) {
 			var key = key_and_values[0]
 			var style = new Style(key_and_values[1])
 			style_map[key] = style
-			
 			elem = create_master_element(key, style.color, '', select_color_master_element)
 			if (first) {
 				/* It's the first element, so select it, so something gets selected */
@@ -630,10 +629,27 @@ function picked_colorpicker_target(tab) {
 	reflect_style()
 }
 
+/* Given a color name, like 'normal' or 'red' or 'FF00F0', return an RGB color string (or empty string) */
+function interpret_color(str) {
+	str = str.toLowerCase()
+	if (str == 'black') return '000000'
+	if (str == 'red') return 'FF0000'
+	if (str == 'green') return '00FF00'
+	if (str == 'brown') return '725000'
+	if (str == 'yellow') return 'FFFF00'
+	if (str == 'blue') return '0000FF'
+	if (str == 'magenta') return 'FF00FF'
+	if (str == 'purple') return 'FF00FF'
+	if (str == 'cyan') return '00FFFF'
+	if (str == 'white') return 'FFFFFF'
+	if (str == 'normal') return ''
+	return str
+}
+
 /* Class representing a color style */
 function Style(stuff) {
-	this.color = stuff[0]
-	this.background_color = stuff[1]
+	this.color = interpret_color(stuff[0])
+	this.background_color = interpret_color(stuff[1])
 	this.bold = stuff[2]
 	this.underline = stuff[3]
 }
@@ -887,15 +903,13 @@ term256_colors = [ //247
 var items_per_row = 15
 var show_labels = 0
 
-var COLOR_NORMAL = 'DDDDDD'
+var COLOR_NORMAL = 'CCC'
 
 /* Adds a new element to master */
 function create_master_element(contents, color, font_size, click_handler) {
-	if (color.length == 0) color = 'inherit'
-	
 	/* In the master list, ensure the color is visible against the dark background */
-	master_color = master_color_for_color(color)
-	style_str = 'color: #' + master_color + '; border-bottom: 1px solid #' + master_color + ' ;'
+	var master_color = color ? master_color_for_color(color) : COLOR_NORMAL
+	var style_str = 'color: #' + master_color + '; border-bottom: 1px solid #' + master_color + ' ;'
 	
 	if (font_size.length > 0) {
 		style_str += 'font-size: ' + font_size + ';'
diff --git a/share/tools/web_config/webconfig.py b/share/tools/web_config/webconfig.py
index c0faddb6..b73a19ce 100755
--- a/share/tools/web_config/webconfig.py
+++ b/share/tools/web_config/webconfig.py
@@ -88,11 +88,37 @@ class FishConfigHTTPRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler):
 	def do_get_colors(self):
 		"Look for fish_color_*"
 		result = []
+		remaining = set(['normal',
+						 'error',
+						 'command',
+						 'end',
+						 'param',
+						 'comment',
+						 'match',
+						 'search_match',
+						 'operator',
+						 'escape',
+						 'quote',
+						 'redirection',
+						 'valid_path',
+						 'autosuggestion'
+						 ])
+    
 		out, err = run_fish_cmd('set -L')
 		for line in out.split('\n'):
 			for match in re.finditer(r"^fish_color_(\S+) ?(.*)", line):
-				color_name, color_value = match.group(1, 2)
-				result.append([color_name.strip(), parse_color(color_value)])
+				color_name, color_value = [x.strip() for x in match.group(1, 2)]
+				result.append([color_name, parse_color(color_value)])
+				remaining.discard(color_name)
+				
+		# Ensure that we have all the color names we know about, so that if the
+		# user deletes one he can still set it again via the web interface
+		for x in remaining:
+			result.append([x, parse_color('')])
+			
+		# Sort our result (by their keys)
+		result.sort()
+		
 		return result
 		
 	def do_get_functions(self):