17 add support for multiple require statements - Githubissues

anacierdem / vscode-requirejs

Provides goto definition functionality for require js modules.

MIT License

21 stars 14 forks source link

17 add support for multiple require statements #31

Closed michaelw85 closed 7 years ago

michaelw85 commented 7 years ago

Updated version of pull request created before to support multiple require statements. Added lots of unit tests + improved functionality.

Manually tested every test file, passed every manual test I did.

Ignores comments now.
Support require().func statements
Supports multi line module path definition
Added eslint validation on build (travis-ci)

anacierdem commented 7 years ago

Support require().func statements This was already fixed on master, fyi.

michaelw85 commented 7 years ago

I know but after merging master into my branch it did not work for the test files so I had to add some logic.

anacierdem commented 7 years ago

test7.js cannot go to definition on moduleAget a console error

Cannot read property 'range' of undefined: TypeError: Cannot read property 'range' of undefined at from (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/node/extensionHostProcess.js:23:273133) at Array.map (native) at /Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/node/extensionHostProcess.js:23:350159 at Object.m [as _notify] (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/node/extensionHostProcess.js:23:41973) at Object.enter (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/node/extensionHostProcess.js:23:45538) at n.Class.derive._oncancel._run (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/node/extensionHostProcess.js:23:47366) at n.Class.derive._oncancel._completed (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/node/extensionHostProcess.js:23:46807) at process._tickCallback (internal/process/next_tick.js:103:7) e.onUnexpectedError @ /Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/electron-browser/workbench.main.js:29

anacierdem commented 7 years ago

Will test and review again soon.

anacierdem commented 7 years ago

See my new comments. Also, console error still there Cannot read property 'range' of undefined: TypeError:

michaelw85 commented 7 years ago

Pushed new changes resolving test 10.

Added check to prevent error when file cannot be found (resulting in range of undefined error)

anacierdem commented 7 years ago

Still does not work properly for a realistic module file.

anacierdem commented 7 years ago

I have developed the alternative solution, please check it. I suggest you allocate your resources for writing tests (after fixing current tests) for the new features. Please write your opinion on this.

anacierdem commented 7 years ago

I'm closing this pr as it has become too complicated for a simple solution. If you feel comfortable with your changes you can send a new one. Fixing and writing tests for the latest changes is appreciated instead.