search

jeancroy / fuzz-aldrin-plus

Sublime text like fuzzy filtering - compatible with atom/fuzzaldrin
MIT License
269 stars 21 forks source link

support browserify-lite #31

Closed pravi closed 7 years ago

pravi commented 7 years ago

We are packaging fuzzaldrin-plus in debian as it is a dependency for gitlab. We would like to browserify it using tools available in debian. But currently we only have browserify-lite (though we hope to package webpack if we are successful in our corwd funding https://www.generosity.com/community-fundraising/debian-browserify-2) in debian. We could browserify fuzzaldrin-plus with browserify-lite, but it does not work when used in gitlab. See https://gitlab.com/gitlab-org/gitlab-ce/issues/14450

It would be great if you can support using browserify-lite instead of the full browserify suite.

jeancroy commented 7 years ago

Hi Pravi. What build system do you use on gitlab ? Basically I have no external dependency and only need to bundle the require. So webpack, rollup, whatever, are all fine with me. I can investigate browserify-lite but you say it doesn't work out of the box ?

pravi commented 7 years ago

Error we get in chromium web console is 'Uncaught ReferenceError: process is not defined'. Though https://nodejs.org/api/process.html says its "The process object is a global that provides information about, and control over, the current Node.js process. As a global, it is always available to Node.js applications without using require()."

pravi commented 7 years ago

nodejs version is 4.3.1. Could that be an issue?

jeancroy commented 7 years ago

So I have two usage of process

https://github.com/jeancroy/fuzzaldrin-plus/blob/da2cb58b7a2a21329f845c7f68d5ad7c12462d71/src/fuzzaldrin.coffee#L8

and

https://github.com/jeancroy/fuzzaldrin-plus/blob/da2cb58b7a2a21329f845c7f68d5ad7c12462d71/benchmark/benchmark.coffee#L23

First one I think I check that process exist, second one is benchmark so should not be bundled. I can change to compare process with undefined, maybe that'll work better.

jeancroy commented 7 years ago

I'll release a version once I handle the bower support and review the nugget #30 should be soon. I'll see if I can use -lite by default too.

pravi commented 7 years ago

@jeancroy would 'process' variable be available in browser environment too? Shouldn't browserify-lite convert this too? For now I have hard coded defaultPathSeparator = '/' as this code is expected to run only on debian.

jeancroy commented 7 years ago

browserify-lite project page state that No builtin Node.js shims. without shim, process is available on nodejs but not browser.

I'll also point that the path separator is configurable, it is probably the best choice on browser.

pravi commented 7 years ago

Would this fail on Windows if I hard code to '/'? What other option is there to find the path separator in the browser environment?

pravi commented 7 years ago

Can we use this https://www.essentialobjects.com/doc/jsdoc.public.web.navigator.getpathseparator.aspx?

jeancroy commented 7 years ago

OK I'll take one step back. This is used in scoring, amongst other things to focus on filename and so entries that are shallow are preferred.

Fundamentally it's a sensible guess on the format of the candidate list. Are file like c:\folder\folder\file.ext or /usr/bin/something.so

Would this fail on Windows if I hard code to '/'?

Sort-of, on path entries the format c:\folder\folder\file.ext, everything will be treated as a giant file name, instead of having an understanding of path, extra point on file name match, and prefer shallower entries. If you specify path separator on the option hash you can disregard default completely.

As discussed before, if you use in a browser with a server generated file list, the os of the browser should not matter.