atom / snippets

Atom snippets package
MIT License
205 stars 100 forks source link

Make the bundled snippets work when the package is snapshotted #244

Closed segevfiner closed 6 years ago

segevfiner commented 6 years ago

Description of the Change

Add the function getPackageRoot and use it to calculate the path to snippets.{cson,json}.

I had to manually apply the changes to an Atom clone to test (I want it packaged in asar, snapshotted), by running script/bootstrap, copying the changes, and commenting out the bootstrap line in script/build. Maybe there is a better way to build Atom with a different private Git copy of a package, but it seems like packageDependencies doesn't support Git urls, and there is no obvious way to execute apm to install a Git package into a local Atom Git clone node_modules directory.

Alternate Designs

Such issues likely repeat them self throughout the builtin Atom packages. Extracting such logic to a shared location and reusing it might help. Note that the exact logic required slightly different from case to case. Some resources are packaged in the asar archive, some are asar.unpacked, some are specially copied in the Atom build scripts. And each such case requires a different function.

Benefits

Builtin snippets such as snip will work again.

Possible Drawbacks

Yet another "calculate the right path" function.

Applicable Issues

Fixes #243

ungb commented 6 years ago

Was able to verify with this change that you can now see snip in snippets.cson and prior to this fix it didn't work. That was the example used in #243

Although I've seen snippets from javascript language package still works before this change.

/cc @as-cii for review.

segevfiner commented 6 years ago

Was able to verify with this change that you can now see snip in snippets.cson and prior to this fix it didn't work. That was the example used in #243

Although I've seen snippets from javascript language package still works before this change.

/cc @as-cii for review.

Only the snippets from lib/snippets.cson should be affected by this change.

as-cii commented 6 years ago

Nice work, thanks! ⚡️