Merge pull request #974 from atom/wl-settings-validation

Validate settings

Author: Wliu

lib/settings-panel.js

@@ -197,6 +197,22 @@ export default class SettingsPanel extends CollapsibleSectionPanel {
     }
   }
 
+  setText (editor, name, type, value) {
+    let stringValue
+    if (this.isDefault(name)) {
+      stringValue = ''
+    } else {
+      stringValue = this.valueToString(value) || ''
+    }
+
+    if (stringValue === editor.getText() || _.isEqual(value, this.parseValue(type, editor.getText()))) {
+      return
+    }
+
+    editor.setText(stringValue)
+    editor.moveToEndOfLine()
+  }
+
   bindSelectFields () {
     const disposables = Array.from(this.element.querySelectorAll('select[id]')).map((select) => {
       const name = select.id
@@ -213,7 +229,6 @@ export default class SettingsPanel extends CollapsibleSectionPanel {
 
   bindEditors() {
     const disposables = Array.from(this.element.querySelectorAll('atom-text-editor')).map((editorElement) => {
-      let left
       let editor = editorElement.getModel()
       let name = editorElement.id
       let type = editorElement.getAttribute('type')
@@ -242,22 +257,21 @@ export default class SettingsPanel extends CollapsibleSectionPanel {
       subscriptions.add(new Disposable(() => editorElement.removeEventListener('blur', blurHandler)))
 
       this.observe(name, (value) => {
-        let stringValue
-        if (this.isDefault(name)) {
-          stringValue = ''
-        } else {
-          stringValue = (left = this.valueToString(value)) != null ? left : ''
-        }
-
-        if (stringValue === editor.getText() || _.isEqual(value, this.parseValue(type, editor.getText()))) {
-          return
-        }
-
-        editor.setText(stringValue)
+        this.setText(editor, name, type, value)
       })
 
       subscriptions.add(editor.onDidStopChanging(() => {
-        this.set(name, this.parseValue(type, editor.getText()))
+        const {minimum, maximum} = atom.config.getSchema(name)
+        const value = this.parseValue(type, editor.getText())
+        if (minimum != null && value < minimum) {
+          this.set(name, minimum)
+          this.setText(editor, name, type, minimum)
+        } else if (maximum != null && value > maximum) {
+          this.set(name, maximum)
+          this.setText(editor, name, type, maximum)
+        } else if (!this.set(name, value)) {
+          this.setText(editor, name, type, atom.config.get(name))
+        }
       }))
 
       return subscriptions

spec/settings-panel-spec.coffee

@@ -214,3 +214,54 @@ describe "SettingsPanel", ->
       expect(controlGroups[1].querySelector('.sub-section .sub-section-heading').classList.contains('has-items')).toBe true
       # Should be already collapsed
       expect(controlGroups[1].querySelector('.sub-section .sub-section-heading').parentElement.classList.contains('collapsed')).toBe true
+
+  describe 'settings validation', ->
+    beforeEach ->
+      config =
+        type: 'object'
+        properties:
+          minMax:
+            name: 'minMax'
+            title: 'Min max'
+            description: 'The minMax setting'
+            type: 'integer'
+            default: 10
+            minimum: 1
+            maximum: 100
+
+      atom.config.setSchema('foo', config)
+      settingsPanel = new SettingsPanel({namespace: 'foo', includeTitle: false})
+
+    it 'prevents setting a value below the minimum', ->
+      minMaxEditor = settingsPanel.element.querySelector('[id="foo.minMax"]')
+      minMaxEditor.getModel().setText('0')
+      advanceClock(minMaxEditor.getModel().getBuffer().getStoppedChangingDelay())
+      expect(minMaxEditor.getModel().getText()).toBe '1'
+
+      minMaxEditor.getModel().setText('-5')
+      advanceClock(minMaxEditor.getModel().getBuffer().getStoppedChangingDelay())
+      expect(minMaxEditor.getModel().getText()).toBe '1'
+
+    it 'prevents setting a value above the maximum', ->
+      minMaxEditor = settingsPanel.element.querySelector('[id="foo.minMax"]')
+      minMaxEditor.getModel().setText('1000')
+      advanceClock(minMaxEditor.getModel().getBuffer().getStoppedChangingDelay())
+      expect(minMaxEditor.getModel().getText()).toBe '100'
+
+      minMaxEditor.getModel().setText('10000')
+      advanceClock(minMaxEditor.getModel().getBuffer().getStoppedChangingDelay())
+      expect(minMaxEditor.getModel().getText()).toBe '100'
+
+    it 'prevents setting a value that cannot be coerced to the correct type', ->
+      minMaxEditor = settingsPanel.element.querySelector('[id="foo.minMax"]')
+      minMaxEditor.getModel().setText('"abcde"')
+      advanceClock(minMaxEditor.getModel().getBuffer().getStoppedChangingDelay())
+      expect(minMaxEditor.getModel().getText()).toBe '' # aka default
+
+      minMaxEditor.getModel().setText('15')
+      advanceClock(minMaxEditor.getModel().getBuffer().getStoppedChangingDelay())
+      expect(minMaxEditor.getModel().getText()).toBe '15'
+
+      minMaxEditor.getModel().setText('"abcde"')
+      advanceClock(minMaxEditor.getModel().getBuffer().getStoppedChangingDelay())
+      expect(minMaxEditor.getModel().getText()).toBe '15'