Fix isTestFile, ITE race, hover-tip cleanup, and dialog Enter key (#354)

Add cd.isTestFile() to identify files whose stem starts or ends with
test/tests/spec/steps. Use it to colour filenames steel-blue in the
review diff panel and hover-tips, and to simplify inter-test event
icons to file_test/file_code.

Fix a race where clicking a filename fired saveAnyFileChangesITE
asynchronously, allowing a concurrent create/delete/rename/test to
use a stale kata.index. The fix locks all CodeMirror editors and
gates create/delete/rename/test behind cd.waitForITE(), which polls
up to 2s for the in-flight ITE to settle.

Clean up hover-tips: remove the traffic-light image and type-marker
glyph column; colour filenames by file type.

Fix Enter key in the delete file dialog, where the disabled input
blocked the keyup handler. Handling moved to dialog-level keydown
with event.preventDefault() to suppress browser default submit.

Author: JonJagger

source/app/assets/javascripts/cyber-dojo_codemirror.js

@@ -77,6 +77,14 @@ var cyberDojo = ((cd, $) => {
 
   //- - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 
+  cd.setFilesEditable = (editable) => {
+    $.each($('.CodeMirror'), (i, editorDiv) => {
+      editorDiv.CodeMirror.setOption('readOnly', !editable);
+    });
+  };
+
+  //- - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
   cd.saveCodeFromIndividualSyntaxHighlightEditor = (filename) => {
     const editorDiv = document.getElementById(syntaxHighlightFileContentForId(filename));
     editorDiv.CodeMirror.cyberDojoTextArea.value = editorDiv.CodeMirror.getValue();

source/app/assets/javascripts/cyber-dojo_hover_tips.js

@@ -25,7 +25,6 @@ var cyberDojo = (function(cd, $) {
   const $trafficLightSummary = (light, kataId) => {
     const $tr = $('<tr>');
     $tr.append($trafficLightIndexTd(light));
-    $tr.append($trafficLightImageTd(light));
     $tr.append($('<td class="mini-text">').html(miniTextInfo(kataId, light)));
     return $('<table>').append($tr);
   };
@@ -37,14 +36,6 @@ var cyberDojo = (function(cd, $) {
     return $('<td>').append($count);
   };
 
-  const $trafficLightImageTd = (light) => {
-    const $img = $('<img>', {
-        src:`/images/traffic-light/${light.colour}.png`,
-      class:'diff-hover-tip'
-    });
-    return $('<td>').append($img);
-  };
-
   const miniTextInfo = (kataId, light) => {
     switch (light.colour) {
       case 'create':      return 'Kata created';
@@ -122,7 +113,6 @@ var cyberDojo = (function(cd, $) {
         const $tr = $('<tr>');
         $tr.append($lineCountTd('deleted', fileDiff));
         $tr.append($lineCountTd('added', fileDiff));
-        $tr.append($diffTypeTd(fileDiff));
         $tr.append($diffFilenameTd(fileDiff));
         $table.append($tr);
       }
@@ -145,17 +135,12 @@ var cyberDojo = (function(cd, $) {
     return $('<td>').append($count);
   };
 
-  const $diffTypeTd = (diff) => {
-    const $type = $('<div>', {
-      class:`hover diff-type-marker ${diff.type}`
-    });
-    return $('<td>').append($type);
-  };
-
   const $diffFilenameTd = (diff) => {
-    const $filename = $('<div>', { class:`hover diff-filename ${diff.type}` });
-    $filename.text(diffFilename(diff));
-    return $('<td>').append($filename);
+    const filename = diffFilename(diff);
+    const fileType = cd.isTestFile(filename) ? 'test-file' : 'non-test-file';
+    const $filename = $('<div>', { class:`hover diff-filename ${diff.type} ${fileType}` });
+    $filename.text(filename);
+    return $('<td>', { style:'padding-left:4px' }).append($filename);
   };
 
   const diffFilename = (diff) => {

source/app/assets/javascripts/cyber-dojo_lib_append_traffic_light.js

@@ -62,10 +62,12 @@ var cyberDojo = ((cd, $) => {
 
   // - - - - - - - - - - - - - - - - - - - - - - - - - - - -
   const $fileEventImage = (event) => {
+    const filename = event.colour === 'file_rename' ? event.new_filename : event.filename;
+    const icon = cd.isTestFile(filename) ? 'file_test' : 'file_code';
     return $('<img>', {
       class: 'diff-traffic-light',
-        src: `/images/traffic-light/${event.colour}.png`,
-        alt: `${event.colour} traffic-light`,
+        src: `/images/traffic-light/${icon}.png`,
+        alt: icon,
       'data-colour': event.colour,
       'data-index': event.index
     });

source/app/assets/stylesheets/hover-tip.scss

@@ -22,6 +22,9 @@ $hoverFadePercent: 5%;
   width: auto; min-width: 0; // reset width
 }
 
+.hover-tip .diff-filename.test-file     { color: steelblue; }
+.hover-tip .diff-filename.non-test-file { color: white; }
+
 .hover-tip .mini-text
 {
   color: lighten(Gray,10%);

source/app/views/kata/_file_create_rename_delete.erb

@@ -24,9 +24,9 @@ $(() => {
 
   const currentFilename = () => kata.tabs.filename().text();
 
-  cd.createFileButton().click(() => openCreateFileDialog());
-  cd.deleteFileButton().click(() => openDeleteFileDialog());
-  cd.renameFileButton().click(() => openRenameFileDialog());
+  cd.createFileButton().click(() => cd.waitForITE(openCreateFileDialog));
+  cd.deleteFileButton().click(() => cd.waitForITE(openDeleteFileDialog));
+  cd.renameFileButton().click(() => cd.waitForITE(openRenameFileDialog));
 
   if (!cd.kata.isLatest()) {
     cd.createFileButton().prop('disabled', true);
@@ -117,10 +117,14 @@ $(() => {
     $(dialog).on('close', () => { dialog.remove(); cd.kata.editor.refocus(); });
     $('.dialog-close', dialog).click(() => dialog.close());
 
-    input.keyup((event) => {
+    input.keyup(() => {
       const newFilename = $.trim(input.val());
       $okButton.prop('disabled', !isValidFilename(newFilename));
-      if (!$okButton.prop('disabled') && event.key === 'Enter') {
+    });
+
+    $(dialog).keydown((event) => {
+      if (event.key === 'Enter' && !$okButton.prop('disabled')) {
+        event.preventDefault();
         $okButton.click();
       }
     });
@@ -132,6 +136,14 @@ $(() => {
 
   // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
   const filenameRange = (f) => {
+    // Returns [lo,hi] for setSelectionRange, pre-selecting the meaningful part
+    // of the filename in the create/rename dialog. Narrows the selection by:
+    // 1. Stripping the extension (.py, .js etc) from the right.
+    // 2. Stripping any leading directory path (src/ test/ etc) from the left.
+    // 3. Stripping any test word (test/tests/spec/steps) plus an adjacent
+    //    separator (. _ - or nothing) from either end of what remains.
+    // eg src/hiker_test.py -> selects only "hiker".
+
     let lo = 0;
     // remove trailing .extension
     const ext = f.lastIndexOf('.');

source/app/views/kata/_file_inter_test_events.erb

@@ -4,6 +4,29 @@ document.addEventListener('DOMContentLoaded', () => {
 
   const kata = cd.kata;
 
+  // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
+  let _interTestEventInProgress = false;
+
+  cd.interTestEventInProgress = () => _interTestEventInProgress;
+
+  cd.waitForITE = (callback) => {
+    if (!_interTestEventInProgress) {
+      callback();
+      return;
+    }
+    const pollInterval = 50;
+    const maxWait = 2000;
+    let elapsed = 0;
+    const poll = setInterval(() => {
+      elapsed += pollInterval;
+      if (!_interTestEventInProgress || elapsed >= maxWait) {
+        clearInterval(poll);
+        callback();
+      }
+    }, pollInterval);
+  };
+
   // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
   // Inter-Test-Events
   // Called in many places, eg when a file is created, deleted, renamed, 
@@ -30,8 +53,12 @@ document.addEventListener('DOMContentLoaded', () => {
 
   cd.saveAnyFileChangesITE = (callback) => {
     cd.saveCodeFromSyntaxHighlightEditors();
+    _interTestEventInProgress = true;
     const args = { id: kata.id, index: kata.index };
-    syncPostWithCallbackITE('/kata/file_edit', args, callback);
+    syncPostWithCallbackITE('/kata/file_edit', args, () => {
+      _interTestEventInProgress = false;
+      callback();
+    });
   };
 
   const syncPostWithCallbackITE = (url, args, callback) => {

source/app/views/kata/_filenames.erb

@@ -125,8 +125,14 @@ $(() => {
       $div.addClass('source');
     }
     $div.click(() => {
+      cd.setFilesEditable(false);
+      try {
+        cd.saveAnyFileChangesITE(() => { cd.setFilesEditable(true); });
+      } catch(e) {
+        cd.setFilesEditable(true);
+        throw e;
+      }
       filenames.select(filename);
-      cd.saveAnyFileChangesITE(() => {});
     });
     return $div;
   };

source/app/views/kata/_test_button.erb

@@ -8,7 +8,7 @@ $(() => {
 
   const $button = $('#test-button');
 
-  $button.click(() => kata.testButton.click());
+  $button.click(() => cd.waitForITE(() => kata.testButton.click()));
 
   kata.testButton = {
     hide: (f) => {

source/app/views/shared/_sorted_filenames.erb

@@ -38,6 +38,17 @@ $(() => {
       filename != 'cyber-dojo.sh';
   };
 
+  cd.isTestFile = (filename) => {
+    // True if the stem starts or ends with a test word (tests/test/spec/steps).
+    // Middle-only matches (eg my_test_helper) do not count.
+    const testWords = ['tests', 'test', 'spec', 'steps'];
+    const ext = filename.lastIndexOf('.');
+    const hi = (ext !== -1) ? ext : filename.length;
+    const stem = filename.substring(0, hi).toLowerCase();
+    const base = stem.substring(stem.lastIndexOf('/') + 1);
+    return testWords.some(word => base.startsWith(word) || base.endsWith(word));
+  };
+
   cd.highlightFilenames = () => {
     const manifest = cd.kata.manifest();
     return manifest.highlight_filenames;