-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make metadata file locations clearer #5
Comments
👍 |
👍 |
👍 |
👍 I too came looking for this after failing miserably. |
Which also begs the question... is that desireable? |
Agreed, I spent quite some time debugging this. |
Can I propose the following behaviour:
Hopefully intuitive/in-alignment with how require statements work etc. ? As it's such a tiny script I'd love to have a crack at a PR as soon as I get a sec, just putting this out there for anyone else coming across this or for thoughts from you guys who have already been burned? EDIT: Only bummer with this is this is actually backwards incompat. with the example in the current README, which uses Perhaps just passing an option: |
Just hit this problem like everybody else. Could we get this issue addressed (with the readme also noting that the leading |
I'm hitting this same problem, but at the moment the behavior between WIN/OSX is not consistent(?!). Works on OSX, does not appear to work on win... |
@nilliams |
this needs to be resolve ASAP! |
Yea, the inconsistency between OSs is really holding things back for my team. |
Only way I got it to work (on Windows) was to place my json files inside de Tried absolute, relative with and without the Debugged the current working directory in the plugin, and it's pointing to the root of the project, so I have no idea why this won't work. |
So I'm not totally sure if this solves the issues with regards to the Windows inconsistency, but I made a fork of the plugin to at least deal with loading data from files outside of the https://github.com/JemBijoux/metalsmith-metadata/tree/ext-data-sources
Once the files are sourced they're included on the metadata just like before, so all this really does is provide a means of loading other data files outside those included in the metalsmith source ( I can't test on Windows myself, but I wonder if making use of the option could help folks having difficulty between OS's (as it makes use of a more familiar node path style)? Any feedback is welcome, let me know if this is helpful to anyone. |
👍 👍 👍 |
👍 |
👍 👍 👍 |
This has been open for a while. Does this change request feel appropriate to the project? If so, happy to provide a PR.. |
I still can't make the path work. After inspecting the code and trying every possible option (except clearly the one that works). I'm about to rewrite this functionality from scratch just so I can make it work. But would rather it work out of the box. I did a path.resolve(file) directly in this modules index.js and it points straight to my file. Likewise, my file is in the metalsmith files hash and using that key also did not work. As far as I'm concerned this module is broken. Okay, I got it to work, doing "exactly" what I had already done, that is using the files hash value as the path-key. Still, this needs to be documented and a decent example provided to illustrate how to map the paths. |
Had similar issue on Windows. Kept getting |
Version 1.0.0 assumes that all metadata files/directories are located in the Metalsmith root directory. Please open a new issue with any new problems. |
It would be good if the Readme made it clearer that the metadata files in question need to be loaded in the metalsmith 'tree', e.g. in your
src
directory if using default options. Took me a while to figure this out - had to look at the source.As you invoke metalsmith on
__dirname
I think you could reasonably expect it to find metadata files relative to there (I did).You might also expect absolute file paths to work (perhaps if invoking from some higher-level tool, gulp etc) and of course they don't for the same reason. I think this is reasonable, but should probably be noted as well.
Thanks!
The text was updated successfully, but these errors were encountered: