From 0fe9994af0e0e547bc845a010698ccdb36303653 Mon Sep 17 00:00:00 2001 From: Henric Trotzig Date: Mon, 4 Jan 2016 11:32:03 +0100 Subject: [PATCH 1/4] Revert "Warn on plugin load if executable missing or wrong version" This reverts commit 0f7d1314eab07f2f7948d2897bde2139673ac153. It turns out that Package control bundles the plugin in a single file called ImportJS.sublime-package. So checking for files won't really work unless we unpack the plugin. Which we don't want. I'm going to mention having to `gem update import_js` any time a new version is installed instead. --- import-js.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/import-js.py b/import-js.py index eb0878c5..a4968097 100644 --- a/import-js.py +++ b/import-js.py @@ -1,23 +1,5 @@ import sublime, sublime_plugin, subprocess, os, json -def plugin_loaded(): - settings = sublime.load_settings('ImportJS.sublime-settings') - executable = os.path.expanduser(settings.get('executable')) - try: - version = subprocess.check_output( - [executable, '--version']) .decode('UTF-8').strip() - except FileNotFoundError: - print(no_executable_error(executable)) - - version_rb = os.path.join(os.path.dirname(__file__), 'lib/import_js/version.rb') - with open(version_rb) as f: - contents = f.read() - - if("VERSION = '" + version + "'" not in contents): - print('The installed version of import-js does not match ' + - 'the current version: ' + version) - - def no_executable_error(executable): return ( "Couldn't find executable " From 420a557a2c0357eb35addb9edd88e5bf955f1c3a Mon Sep 17 00:00:00 2001 From: Henric Trotzig Date: Mon, 4 Jan 2016 11:51:17 +0100 Subject: [PATCH 2/4] Merge test for import_all and remove_unused_imports I'm about to optimize fix_imports by only doing one call to eslint, and one call to update imports. Before I do that, I wanted to make sure that fix_imports was tested. Which we kind of did, but the two methods separately. --- spec/import_js/importer_spec.rb | 52 ++++++++++++++++----------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/spec/import_js/importer_spec.rb b/spec/import_js/importer_spec.rb index e9b027f0..3129102e 100644 --- a/spec/import_js/importer_spec.rb +++ b/spec/import_js/importer_spec.rb @@ -1191,7 +1191,7 @@ def self.current_selection=(index) end end - describe '#import_all' do + describe '#fix_imports' do let(:eslint_result) { '' } before do allow(Open3).to receive(:capture3).and_call_original @@ -1200,7 +1200,7 @@ def self.current_selection=(index) end subject do - ImportJS::Importer.new.import_all + ImportJS::Importer.new.fix_imports VIM::Buffer.current_buffer.to_s end @@ -1312,20 +1312,6 @@ def self.current_selection=(index) EOS end end - end - - describe '#remove_unused_imports' do - let(:eslint_result) { '' } - before do - allow(Open3).to receive(:capture3).and_call_original - allow(Open3).to receive(:capture3).with(/eslint/, anything) - .and_return([eslint_result, nil]) - end - - subject do - ImportJS::Importer.new.remove_unused_imports - VIM::Buffer.current_buffer.to_s - end context 'when no unused variables exist' do it 'leaves the buffer unchanged' do @@ -1333,17 +1319,7 @@ def self.current_selection=(index) end end - context 'when eslint can not parse' do - let(:eslint_result) do - 'stdin:1:1: Parsing error: Unexpected token ILLEGAL [Error]' - end - - it 'throws an error' do - expect { subject }.to raise_error(ImportJS::ParseError) - end - end - - context 'when one unused variable exists' do + context 'when one unused import exists' do let(:text) { <<-EOS.strip } var bar = require('foo/bar'); var foo = require('bar/foo'); @@ -1386,6 +1362,28 @@ def self.current_selection=(index) end end + context 'when an unused import and an undefined variable exists' do + let(:existing_files) { ['bar/foo.jsx'] } + let(:text) { <<-EOS.strip } +var bar = require('foo/bar'); + +foo + EOS + + let(:eslint_result) do + "stdin:3:11: \"bar\" is defined but never used [Error/no-unused-vars]\n" \ + "stdin:3:11: \"foo\" is not defined. [Error/no-undef]" + end + + it 'removes the unused import and adds the missing one' do + expect(subject).to eq(<<-EOS.strip) +var foo = require('bar/foo'); + +foo + EOS + end + end + context 'when a destructured import has an unused variable' do let(:text) { <<-EOS.strip } var { bar, foo } = require('baz'); From 46fcb8f8e647f6bce7a2b592bc2a6ed8442489d1 Mon Sep 17 00:00:00 2001 From: Henric Trotzig Date: Mon, 4 Jan 2016 12:09:20 +0100 Subject: [PATCH 3/4] Remove support for `import_all` and `remove_unused_imports` @kevinkehl pointed out that it was confusing that `import_all` and `fix_imports` both existed. I agree. To remove some confusion, I'm removing `import_all`. `fix_imports` has the same dependencies (eslint), and after improving performance (making sure that only one eslint call is made and that the buffer is only changed once) we might as well go all in and also remove unused imports. --- Default.sublime-commands | 4 ---- README.md | 8 +++++--- SUBLIME.md | 10 +++++----- autoload/importjs.vim | 2 -- bin/import-js | 5 +---- lib/import_js/importer.rb | 34 +++++++++++---------------------- plugin/import-js.vim | 1 - spec/import_js/importer_spec.rb | 7 ------- 8 files changed, 22 insertions(+), 49 deletions(-) diff --git a/Default.sublime-commands b/Default.sublime-commands index 53dac721..afe68fa0 100644 --- a/Default.sublime-commands +++ b/Default.sublime-commands @@ -1,8 +1,4 @@ [ - { - "caption": "ImportJS: import all dependencies", - "command": "import_js" - }, { "caption": "ImportJS: fix all imports", "command": "import_js", diff --git a/README.md b/README.md index 6a6f95fd..f94f2c82 100644 --- a/README.md +++ b/README.md @@ -71,9 +71,11 @@ a `require` statement. But keep reading for some more neat features. ## Fix imports If you have [eslint](http://eslint.org/) installed, import-js can be used to -automatically fix all imports. By hiting `i` (Vim), all your undefined -variables will be resolved, and all your unused imports will be removed. By -default, import-js expects a global `eslint` command to be available. +automatically fix all imports. By hiting `i` (Vim), `(M-x) +import-js-fix` (Emacs), or choose `ImportJS: fix all imports` (Sublime), all +your undefined variables will be resolved, and all your unused imports will be +removed. By default, import-js expects a global `eslint` command to be +available. ## Experimental: Go to module diff --git a/SUBLIME.md b/SUBLIME.md index 92259188..01783096 100644 --- a/SUBLIME.md +++ b/SUBLIME.md @@ -11,13 +11,13 @@ 5. Open the root of your project as a folder (Project -> Add Folder to Project…) 6. Import a file! * Whenever you have undefined variables, open the Command Palette - (CTRL/CMD+SHIFT+P) and select "ImportJS: import all dependencies", - "ImportJS: fix all imports" or "ImportJS: import word under cursor". + (CTRL/CMD+SHIFT+P) and select "ImportJS: fix all imports", + or "ImportJS: import word under cursor". * It will be helpful to bind `import_js` to easy-to-use bindings, such as: ``` - { "keys": ["super+alt+i"], "command": "import_js", "args": {"fix": true} }, - { "keys": ["super+alt+j"], "command": "import_js", "args": {"word": true} }, - { "keys": ["super+alt+g"], "command": "import_js", "args": {"word": true, "goto": true} }, + { "keys": ["super+alt+i"], "command": "import_js" }, + { "keys": ["super+alt+j"], "command": "import_js", "args": { "word": true } }, + { "keys": ["super+alt+g"], "command": "import_js", "args": { "word": true, "goto": true } }, ``` diff --git a/autoload/importjs.vim b/autoload/importjs.vim index c0241f5c..dd73920c 100644 --- a/autoload/importjs.vim +++ b/autoload/importjs.vim @@ -1,8 +1,6 @@ function importjs#ImportJSImport() ruby $import_js.import endfunction -function importjs#ImportJSImportAll() - ruby $import_js.import_all endfunction function importjs#ImportJSGoTo() ruby $import_js.goto diff --git a/bin/import-js b/bin/import-js index 58028643..4bae5079 100755 --- a/bin/import-js +++ b/bin/import-js @@ -7,7 +7,6 @@ require 'json' opts = Slop.parse do |o| o.string '-w', '--word', 'a word/variable to import' o.bool '--goto', 'instead of importing, just print the path to a module' - o.bool '--fix', 'imports all undefined variables, and removes unused imports' o.array '--selections', 'a list of resolved selections, e.g. Foo:0,Bar:1' o.string '--filename', 'a path to the file which contents are being passed in as stdin' @@ -37,10 +36,8 @@ if opts.goto? importer.goto elsif opts[:word] importer.import -elsif opts.fix? - importer.fix_imports else - importer.import_all + importer.fix_imports end if opts.goto? diff --git a/lib/import_js/importer.rb b/lib/import_js/importer.rb index e56d5624..eed12636 100644 --- a/lib/import_js/importer.rb +++ b/lib/import_js/importer.rb @@ -47,36 +47,17 @@ def goto @editor.open_file(js_module.file_path) if js_module end + # Removes unused imports and adds imports for undefined variables def fix_imports - remove_unused_imports - import_all - end - - # Finds all variables that haven't yet been imported. - def import_all @config.refresh - undefined_variables = run_eslint_command.map do |line| + eslint_result = run_eslint_command + undefined_variables = eslint_result.map do |line| /(["'])([^"']+)\1 is not defined/.match(line) do |match_data| match_data[2] end end.compact.uniq - return message('No variables to import') if undefined_variables.empty? - - old_imports = find_current_imports - undefined_variables.each do |variable| - if js_module = find_one_js_module(variable) - inject_js_module(variable, js_module, old_imports[:imports]) - end - end - replace_imports(old_imports[:newline_count], - old_imports[:imports], - old_imports[:imports_start_at]) - end - - def remove_unused_imports - @config.refresh - unused_variables = run_eslint_command.map do |line| + unused_variables = eslint_result.map do |line| /"([^"]+)" is defined but never used/.match(line) do |match_data| match_data[1] end @@ -89,6 +70,13 @@ def remove_unused_imports end import_statement.variables.empty? end + + undefined_variables.each do |variable| + if js_module = find_one_js_module(variable) + inject_js_module(variable, js_module, new_imports) + end + end + replace_imports(old_imports[:newline_count], new_imports, old_imports[:imports_start_at]) diff --git a/plugin/import-js.vim b/plugin/import-js.vim index e88a233c..0395185e 100644 --- a/plugin/import-js.vim +++ b/plugin/import-js.vim @@ -4,7 +4,6 @@ endif let g:import_js_loaded = 1 command ImportJSImport call importjs#ImportJSImport() -command ImportJSImportAll call importjs#ImportJSImportAll() command ImportJSGoTo call importjs#ImportJSGoTo() command ImportJSRemoveUnusedImports call importjs#ImportJSRemoveUnusedImports() command ImportJSFixImports call importjs#ImportJSFixImports() diff --git a/spec/import_js/importer_spec.rb b/spec/import_js/importer_spec.rb index 3129102e..377ac6ab 100644 --- a/spec/import_js/importer_spec.rb +++ b/spec/import_js/importer_spec.rb @@ -1208,13 +1208,6 @@ def self.current_selection=(index) it 'leaves the buffer unchanged' do expect(subject).to eq(text) end - - it 'displays a message' do - subject - expect(VIM.last_command_message).to eq( - 'ImportJS: No variables to import' - ) - end end context 'when eslint can not parse' do From c763e9851726705ed796c041a1e0ba57e58a00ab Mon Sep 17 00:00:00 2001 From: Henric Trotzig Date: Mon, 4 Jan 2016 12:12:47 +0100 Subject: [PATCH 4/4] Bump version 0.1.5 to 0.2.0 Changes: - Removed support for `import_all` (use `fix_imports` instead) --- lib/import_js/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/import_js/version.rb b/lib/import_js/version.rb index 422a5dc0..98aa92d8 100644 --- a/lib/import_js/version.rb +++ b/lib/import_js/version.rb @@ -1,4 +1,4 @@ # Defines the gem version. module ImportJS - VERSION = '0.1.5' + VERSION = '0.2.0' end