Make Accelerator a standalone JS type.

This makes menu and global-shortcut share the same code on accelerator.

atom.gyp

@@ -194,6 +194,8 @@
       'atom/common/draggable_region.cc',
       'atom/common/draggable_region.h',
       'atom/common/linux/application_info.cc',
+      'atom/common/native_mate_converters/accelerator_converter.cc',
+      'atom/common/native_mate_converters/accelerator_converter.h',
       'atom/common/native_mate_converters/file_path_converter.h',
       'atom/common/native_mate_converters/function_converter.h',
       'atom/common/native_mate_converters/gurl_converter.h',

atom/browser/api/atom_api_global_shortcut.cc

@@ -6,8 +6,9 @@
 
 #include <string>
 
-#include "atom/browser/ui/accelerator_util.h"
+#include "atom/common/native_mate_converters/accelerator_converter.h"
 #include "atom/common/native_mate_converters/function_converter.h"
+#include "base/stl_util.h"
 #include "native_mate/dictionary.h"
 
 #include "atom/common/node_includes.h"
@@ -36,52 +37,35 @@ void GlobalShortcut::OnKeyPressed(const ui::Accelerator& accelerator) {
   accelerator_callback_map_[accelerator].Run();
 }
 
-bool GlobalShortcut::Register(const std::string& keycode,
+bool GlobalShortcut::Register(const ui::Accelerator& accelerator,
-    const base::Closure& callback) {
+                              const base::Closure& callback) {
-  ui::Accelerator accelerator;
-  if (!accelerator_util::StringToAccelerator(keycode, &accelerator)) {
-    LOG(ERROR) << keycode << " is invalid.";
-    return false;
-  }
   if (!GlobalShortcutListener::GetInstance()->RegisterAccelerator(
       accelerator, this)) {
     return false;
   }
+
   accelerator_callback_map_[accelerator] = callback;
   return true;
 }
 
-void GlobalShortcut::Unregister(const std::string& keycode) {
+void GlobalShortcut::Unregister(const ui::Accelerator& accelerator) {
-  ui::Accelerator accelerator;
+  if (!ContainsKey(accelerator_callback_map_, accelerator))
-  if (!accelerator_util::StringToAccelerator(keycode, &accelerator)) {
-    LOG(ERROR) << "The keycode: " << keycode << " is invalid.";
     return;
-  }
+
-  if (accelerator_callback_map_.find(accelerator) ==
-      accelerator_callback_map_.end()) {
-    LOG(ERROR) << "The keycode: " << keycode << " isn't registered yet!";
-    return;
-  }
   accelerator_callback_map_.erase(accelerator);
   GlobalShortcutListener::GetInstance()->UnregisterAccelerator(
       accelerator, this);
 }
 
+bool GlobalShortcut::IsRegistered(const ui::Accelerator& accelerator) {
+  return ContainsKey(accelerator_callback_map_, accelerator);
+}
+
 void GlobalShortcut::UnregisterAll() {
   accelerator_callback_map_.clear();
   GlobalShortcutListener::GetInstance()->UnregisterAccelerators(this);
 }
 
-bool GlobalShortcut::IsRegistered(const std::string& keycode) {
-  ui::Accelerator accelerator;
-  if (!accelerator_util::StringToAccelerator(keycode, &accelerator)) {
-    LOG(ERROR) << "The keycode: " << keycode << " is invalid.";
-    return false;
-  }
-  return accelerator_callback_map_.find(accelerator) !=
-      accelerator_callback_map_.end();
-}
-
 // static
 mate::ObjectTemplateBuilder GlobalShortcut::GetObjectTemplateBuilder(
     v8::Isolate* isolate) {

atom/browser/api/atom_api_global_shortcut.h

@@ -34,9 +34,10 @@ class GlobalShortcut : public extensions::GlobalShortcutListener::Observer,
  private:
   typedef std::map<ui::Accelerator, base::Closure> AcceleratorCallbackMap;
 
-  bool Register(const std::string& keycode, const base::Closure& callback);
+  bool Register(const ui::Accelerator& accelerator,
-  bool IsRegistered(const std::string& keycode);
+                const base::Closure& callback);
-  void Unregister(const std::string& keycode);
+  bool IsRegistered(const ui::Accelerator& accelerator);
+  void Unregister(const ui::Accelerator& accelerator);
   void UnregisterAll();
 
   // GlobalShortcutListener::Observer implementation.

atom/browser/api/atom_api_menu.cc

@@ -5,7 +5,7 @@
 #include "atom/browser/api/atom_api_menu.h"
 
 #include "atom/browser/native_window.h"
-#include "atom/browser/ui/accelerator_util.h"
+#include "atom/common/native_mate_converters/accelerator_converter.h"
 #include "atom/common/native_mate_converters/string16_converter.h"
 #include "native_mate/constructor.h"
 #include "native_mate/dictionary.h"
@@ -92,12 +92,7 @@ bool Menu::GetAcceleratorForCommandId(int command_id,
                                                 GetWrapper(isolate),
                                                 "getAcceleratorForCommandId",
                                                 command_id);
-  if (shortcut->IsString()) {
+  return mate::ConvertFromV8(isolate, shortcut, accelerator);
-    std::string shortcut_str = mate::V8ToString(shortcut);
-    return accelerator_util::StringToAccelerator(shortcut_str, accelerator);
-  }
-
-  return false;
 }
 
 bool Menu::IsItemForCommandIdDynamic(int command_id) const {

atom/common/native_mate_converters/accelerator_converter.cc

@@ -0,0 +1,20 @@
+// Copyright (c) 2014 GitHub, Inc. All rights reserved.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "atom/common/native_mate_converters/accelerator_converter.h"
+
+#include "atom/browser/ui/accelerator_util.h"
+
+namespace mate {
+
+// static
+bool Converter<ui::Accelerator>::FromV8(
+    v8::Isolate* isolate, v8::Handle<v8::Value> val, ui::Accelerator* out) {
+  std::string keycode;
+  if (!ConvertFromV8(isolate, val, &keycode))
+    return false;
+  return accelerator_util::StringToAccelerator(keycode, out);
+}
+
+}  // namespace mate

atom/common/native_mate_converters/accelerator_converter.h

@@ -0,0 +1,24 @@
+// Copyright (c) 2014 GitHub, Inc. All rights reserved.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ATOM_COMMON_NATIVE_MATE_CONVERTERS_ACCELERATOR_CONVERTER_H_
+#define ATOM_COMMON_NATIVE_MATE_CONVERTERS_ACCELERATOR_CONVERTER_H_
+
+#include "native_mate/converter.h"
+
+namespace ui {
+class Accelerator;
+}
+
+namespace mate {
+
+template<>
+struct Converter<ui::Accelerator> {
+  static bool FromV8(v8::Isolate* isolate, v8::Handle<v8::Value> val,
+                     ui::Accelerator* out);
+};
+
+}  // namespace mate
+
+#endif  // ATOM_COMMON_NATIVE_MATE_CONVERTERS_ACCELERATOR_CONVERTER_H_

docs/api/accelerator.md

@@ -0,0 +1,41 @@
+# Accelerator
+
+An accelerator is string that represents a keyboard shortcut, it can contain
+multiple modifiers and key codes, combined by the `+` character.
+
+Examples:
+
+* `Command+A`
+* `Ctrl+Shift+Z`
+
+## Platform notice
+
+On Linux and Windows, the `Command` key would not have any effect, you can
+use `CommandOrControl` which represents `Command` on OS X and `Control` on
+Linux and Windows to define some accelerators.
+
+## Available modifiers
+
+* `Command` (or `Cmd` for short)
+* `Control` (or `Ctrl` for short)
+* `CommandOrControl` (or `CmdOrCtrl` for short)
+* `Alt`
+* `Shift`
+
+## Available key codes
+
+* `0` to `9`
+* `A` to `Z`
+* `F1` to `F24`
+* Punctuations like `~`, `!`, `@`, `#`, `$`, etc.
+* `Space`
+* `Backspace`
+* `Delete`
+* `Insert`
+* `Return` (or `Enter` as alias)
+* `Up`, `Down`, `Left` and `Right`
+* `Home` and `End`
+* `PageUp` and `PageDown`
+* `Escape` (or `Esc` for short)
+* `VolumeUp`, `VolumeDown` and `VolumeMute`
+* `MediaNextTrack`, `MediaPreviousTrack`, `MediaStop` and `MediaPlayPause`

docs/api/global-shortcut.md

@@ -22,33 +22,23 @@ globalShortcut.unregister('ctrl+x');
 globalShortcut.unregisterAll();
 ```
 
-## globalShortcut.register(keycode, callback)
+## globalShortcut.register(accelerator, callback)
 
-* `keycode` String
+* `accelerator` [Accelerator](accelerator.md)
 * `callback` Function
 
-Registers a global shortcut of `keycode`, the `callback` would be called when
+Registers a global shortcut of `accelerator`, the `callback` would be called when
 the registered shortcut is pressed by user.
 
-`keycode` is a string to specify shortcut key, such as "ctrl+shift+a".
+## globalShortcut.isRegistered(accelerator)
 
-A `keycode` consists of modifier and key two parts:
+* `accelerator` [Accelerator](accelerator.md)
 
-__Modifiers__: control(ctrl), command(cmd), alt, shift, commandorcontrol(cmdorctrl).
+Returns whether shortcut of `accelerator` is registered.
 
-__Supported keys__: 0-9, a-z, up, down, left, right, home, end, pagedown, pageup,
+## globalShortcut.unregister(accelerator)
-insert, delete, esc, space, backspace, tab, f1-f12, volumeup, volumedown, media 
-keys(medianextrack, mediaprevioustrack, mediastop, mediaplaypause).
 
-## globalShortcut.isRegistered(keycode)
+* `accelerator` [Accelerator](accelerator.md)
-
-* `keycode` String
-
-Return whether the shortcut is registered.
-
-## globalShortcut.unregister(keycode)
-
-* `keycode` String
 
 Unregisters the global shortcut of `keycode`.
 

docs/api/menu-item.md

@@ -12,17 +12,9 @@
      `radio`
   * `label` String
   * `sublabel` String
-  * `accelerator` String - In the form of `Command+R`, `Ctrl+C`,
+  * `accelerator` [Accelerator](accelerator.md)
-    `Shift+Command+D`, `D`, etc.
   * `enabled` Boolean
   * `visible` Boolean
   * `checked` Boolean
   * `submenu` Menu - Should be specified for `submenu` type menu item, when
      it's specified the `type: 'submenu'` can be omitted for the menu item
-
-## Notes on accelerator
-
-On Linux and Windows, the `Command` key would not have any effect, you can
-use `CommandOrControl` which represents `Command` on OS X and `Control` on
-Linux and Windows to define some accelerators, you can also use its short
-alias `CmdOrCtrl`.