Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keymaps #1509

Closed
wants to merge 32 commits into from
Closed

Keymaps #1509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
1bca3ba
Add base Keymaps
ppot Feb 14, 2017
dad0094
add emojs shortcut
ppot Feb 15, 2017
287dc24
remove accelerators
ppot Feb 15, 2017
68865e1
remove menu.js
ppot Feb 15, 2017
7a67ef5
keys removes
ppot Feb 15, 2017
bbc5cf9
mv for asar
ppot Feb 15, 2017
0584894
review
ppot Feb 15, 2017
b1bfe9a
Update plugins installation
ppot Feb 23, 2017
6845bb7
test keymaps as commands
ppot Mar 4, 2017
f78c8b3
Add command-registry
ppot Mar 12, 2017
cf1ab0c
Update command registration
ppot Mar 14, 2017
4c8aa03
remove spaces
ppot Mar 14, 2017
97a30be
mv CommandRegistry inside hterm
ppot Mar 16, 2017
117ddeb
change fuction name
ppot Mar 16, 2017
0ed2e34
remove code comments
ppot Mar 16, 2017
2d272c5
exports _toDependencies
ppot Mar 17, 2017
0b98739
Cleaning
ppot Mar 20, 2017
ffea76b
comments and fnct name change
ppot Apr 25, 2017
7a5139c
SyntaxError handling
ppot Apr 25, 2017
378f5ed
fixed issue where old config file was read as a byte buffer rather th…
MartyGentillon Apr 26, 2017
b53426b
Fixed issue with keymap case sensitivity
MartyGentillon Apr 28, 2017
bfa92d8
simplification
ppot Apr 30, 2017
231451b
Set 1 -> load with default config if errors
ppot Apr 30, 2017
bdea424
Step 2 -> Provide notifications error
ppot Apr 30, 2017
2ba0431
Add keybind for windows:zoom
ppot Apr 30, 2017
33e2333
Remove file
ppot Apr 30, 2017
5f74ab4
Word change
ppot Apr 30, 2017
e680f4f
Add DEV paths for dev testing
ppot May 4, 2017
182d347
const and file rename
ppot May 5, 2017
ddab1d5
update website
ppot May 6, 2017
7cb6f34
rebase
ppot May 13, 2017
41b5699
rename hconf.js to config.js on website
ppot May 14, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 0 additions & 136 deletions app/accelerators.js

This file was deleted.

154 changes: 28 additions & 126 deletions app/config.js
Original file line number Diff line number Diff line change
@@ -1,97 +1,28 @@
const {homedir} = require('os');
const {statSync, renameSync, readFileSync, writeFileSync} = require('fs');
const {resolve} = require('path');
const vm = require('vm');

const {dialog} = require('electron');
const isDev = require('electron-is-dev');
const gaze = require('gaze');
const Config = require('electron-config');
const notify = require('./notify');

// local storage
const winCfg = new Config({
defaults: {
windowPosition: [50, 50],
windowSize: [540, 380]
}
});

let configDir = homedir();
if (isDev) {
// if a local config file exists, use it
try {
const devDir = resolve(__dirname, '..');
const devConfig = resolve(devDir, '.hyper.js');
statSync(devConfig);
configDir = devDir;
console.log('using config file:', devConfig);
} catch (err) {
// ignore
}
}

const path = resolve(configDir, '.hyper.js');
const pathLegacy = resolve(configDir, '.hyperterm.js');
const _import = require('./config/import');
const _paths = require('./config/paths');
const win = require('./config/windows');

const watchers = [];

const scanInterval = 2000;
let cfg = {};

function watch() {
// watch for changes on config every 2s
// windows interval: https://github.com/zeit/hyper/pull/1772
gaze(path, process.platform === 'win32' ? {interval: 2000} : {}, function (err) {
const _watch = function () {
gaze(_paths.prodConf, process.platform === 'win32' ? {interval: scanInterval} : {}, function (err) {
if (err) {
throw err;
}
this.on('changed', () => {
try {
if (exec(readFileSync(path, 'utf8'))) {
notify('Hyper configuration reloaded!');
watchers.forEach(fn => fn());
}
} catch (err) {
dialog.showMessageBox({
message: `An error occurred loading your configuration (${path}): ${err.message}`,
buttons: ['Ok']
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when the configuration fails to parse? It seems that some kind of notification or error dialogue would be useful

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: issue #1589

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added handling of this in SyntaxError

Copy link
Contributor

@MartyGentillon MartyGentillon Apr 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When there is a syntax error in the configuration, the application still fails to start.

App threw an error during load
Error reading configuration: Syntax error found

When that happens, we should probably load the default configuration until the syntax error is resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I thought:
What if instead of copying default config to a file. We use them and overide them only if the user updated is inside his local config.js file?
It will be easier and if error are found we can still lunch hyper and display better notification inside the shell.

Copy link
Contributor

@MartyGentillon MartyGentillon Apr 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One really nice thing about that approach: if there is some new item of configuration, it comes configured by default, which could be really nice. Another thought, perhaps the way to deal with the documentation for custom configuration would be to write the default config into the hyper config directory as an "information" file, along with instructions for generating the config.js.

Something like default_config.js.info with the following disclaimer:

This is the default configuration for hyper.  Modifying this file has no effect.
To override values, add the new value to your config.js file, which overrides the values herein.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rauchg What is your take on this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it easier for users to have a well documented file to modify than copy or create a new one. imho

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really the right place to discuss about it, but maybe it could be a great idea to have a specific section for plugin configuration.
Because of this: chabou/hyper-pane#9 and this chabou/hyper-autoprofile#2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can open an Issue for this since it's a distinct question to be addressed!

}
cfg = _import();
notify('Configuration updated', 'Hyper configuration reloaded!');
watchers.forEach(fn => fn());
});
this.on('error', () => {
// Ignore file watching errors
});
});
}

let _str; // last script
function exec(str) {
if (str === _str) {
return false;
}
_str = str;
const script = new vm.Script(str);
const module = {};
script.runInNewContext({module});
if (!module.exports) {
throw new Error('Error reading configuration: `module.exports` not set');
}
const _cfg = module.exports;
if (!_cfg.config) {
throw new Error('Error reading configuration: `config` key is missing');
}
_cfg.plugins = _cfg.plugins || [];
_cfg.localPlugins = _cfg.localPlugins || [];
cfg = _cfg;
return true;
}

// This method will take text formatted as Unix line endings and transform it
// to text formatted with DOS line endings. We do this because the default
// text editor on Windows (notepad) doesn't Deal with LF files. Still. In 2017.
function crlfify(str) {
return str.replace(/\r?\n/g, '\r\n');
}
};

exports.subscribe = function (fn) {
watchers.push(fn);
Expand All @@ -100,60 +31,31 @@ exports.subscribe = function (fn) {
};
};

exports.init = function () {
// for backwards compatibility with hyperterm
// (prior to the rename), we try to rename
// on behalf of the user
try {
statSync(pathLegacy);
renameSync(pathLegacy, path);
} catch (err) {
// ignore
}

try {
exec(readFileSync(path, 'utf8'));
} catch (err) {
console.log('read error', path, err.message);
const defaultConfig = readFileSync(resolve(__dirname, 'config-default.js'));
try {
console.log('attempting to write default config to', path);
exec(defaultConfig);

writeFileSync(
path,
process.platform === 'win32' ? crlfify(defaultConfig.toString()) : defaultConfig);
} catch (err) {
throw new Error(`Failed to write config to ${path}: ${err.message}`);
}
}
watch();
};

exports.getConfigDir = function () {
// expose config directory to load plugin from the right place
return configDir;
exports.getPlugins = function () {
return {
plugins: cfg.plugins,
localPlugins: cfg.localPlugins
};
};

exports.getConfig = function () {
return cfg.config;
};

exports.getPlugins = function () {
return {
plugins: cfg.plugins,
localPlugins: cfg.localPlugins
};
exports.getKeymaps = function () {
return cfg.keymaps;
};

exports.window = {
get() {
const position = winCfg.get('windowPosition');
const size = winCfg.get('windowSize');
return {position, size};
},
recordState(win) {
winCfg.set('windowPosition', win.getPosition());
winCfg.set('windowSize', win.getSize());
exports.extendKeymaps = function (keymaps) {
if (keymaps) {
cfg.keymaps = keymaps;
}
};

exports.setup = function () {
cfg = _import();
_watch();
};

exports.getWin = win.get;
exports.winRecord = win.recordState;
Loading