brace icon indicating copy to clipboard operation
brace copied to clipboard

Allow using a custom build of Ace

Open tqc opened this issue 7 years ago • 8 comments

Adds the option to build from Ace source rather than the ace-builds repo, which makes it much easier to debug ace in an app that requires brace.

To use the local/custom version of ace, run

npm link /path/to/ace

Note that the output will not be quite the same as obtained from ace-builds, but that appears to be an issue with ace-builds not removing files that no longer exist in the source.

tqc avatar Apr 11 '17 01:04 tqc

I like the idea, but would prefer this being explicit, i.e. --acedir ./ace flag. This way it's obvious instead of magically happening if you happen to have a specific directory in the current folder.

Feel free to pull in a command line tool like minimist to parse the args. Also make sure to pass along any args passed to the npm update script. That way I could do npm run update --acedir ./ace

Thanks.

thlorenz avatar Apr 11 '17 09:04 thlorenz

Adding a full command line for this seems like overkill to me.

Certainly it needs to be documented, but it isn't just putting ace in an arbitrary magic path.

I was going for the conventional behavior of an npm dev/optional dependency - not included with a default npm install, but can be placed in the standard location by running npm install/link and will be used if available.

This approach also opens up possibilities like including ace in devDependencies, though unfortunately ace source isn't published to npm, and npm update is a little flaky with git dependencies.

tqc avatar Apr 12 '17 00:04 tqc

I'm afraid that someone may inadvertently have that folder in their path, possibly from a previous update script run that didn't finish or whatever and then unexpectedly the script uses that without being explicit about it (console.log isn't sufficient). I don't like magic and prefer users to opt in into using a local version.

We don't need minimist necessarily, but at least a flag (you can parse the args manually).

thlorenz avatar Apr 12 '17 09:04 thlorenz

Pretty sure that can't happen. It's in node_modules, so the only way it can legitimately get there is explicitly adding it with npm install or nom link - essentially a config command saying to use this source for all future builds.

Unlike the ace-build folder, it is not created by the script, always manually.

tqc avatar Apr 12 '17 09:04 tqc

Ah ok I think I'm not understanding correctly, you want to build this manually after you installed brace? I though this is so you can create your custom version, publish it and install that.

If the latter is true that's a very special use case isn't it? It's the first time I hear that anyone wants to do that.

makes it much easier to debug ace in an app that requires brace

How so? You're still gonna run the bundled together code, how could you step into the original code?

thlorenz avatar Apr 12 '17 09:04 thlorenz

That was npm install in the dev environment sense - clone brace from git, npm install to get dependencies, then npm install github:tqc/ace or link to add the optional dependency which isn't in package.json.

I wouldn't expect update to be run when using npm install brace from another package, but it does work nicely when using npm link brace so that my dev server gets the customised version of brace.

Using a custom build of ace isn't something many people would need to do, but I happen to need some features / fixes that aren't in the currently released version - specialized, but still a legitimate use case.

I meant debugging in the sense of being able to run it at all without going through the full publish process - sourcemaps are nice, but not essential.

tqc avatar Apr 12 '17 09:04 tqc

This is exactly what I needed. It enabled me to use custom language files. I guess the implementation details just need working out to merge the PR, right?

sstepper avatar Dec 29 '17 00:12 sstepper

This would allow us to use minified ace? How else can we make brace smaller?

https://github.com/fxmontigny/ng2-ace-editor/issues/108#issuecomment-444498685

CAspeling avatar Dec 17 '18 04:12 CAspeling