Skip to content
This repository has been archived by the owner on Feb 24, 2021. It is now read-only.

Files can now be included from other places than source/includes. #47

Merged
merged 3 commits into from
Nov 19, 2018

Conversation

matias-sandell
Copy link
Contributor

Take 2. ;)
Not sure if this is within the scope of Shins, but feel free to use it if you want!

This PR let's a user include files from other places than source/includes if the included file begins with a slash /. The files are included relative to the root shins path and doesn't have to begin with an underscore.

So it's now possible to include file-a.md, file-b.md and file-c.md from shins/myfolder like this;

---
title: MyAPI v1.0.0
includes:
  - /myfolder/file-a
  - /myfolder/file-b
  - /myfolder/file-c
---

If you use widdershins; --includes '/myfolder/file-a,/myfolder/file-b,/myfolder/file-c'.

@matias-sandell
Copy link
Contributor Author

Exactly the same as #46, but from a fresh branch. Sorry for the trouble!

@MikeRalphson
Copy link
Contributor

No trouble, thanks for taking the time.

@MikeRalphson
Copy link
Contributor

LGTM. I'm just thinking we may want to document this somewhere. Because I'm likely to want to build on this with #42 maybe creating a docs/include.md and referencing that from the README would be the best approach. Let me know if you have time, or I can add it.

@MikeRalphson MikeRalphson self-assigned this Nov 19, 2018
@matias-sandell
Copy link
Contributor Author

No problem, I'll add a docs/include.md to my branch later today and include the description of the PR. Then you can do some more edits to it if you wish.

@MikeRalphson
Copy link
Contributor

Thanks, that's great.

@matias-sandell
Copy link
Contributor Author

Done! Please edit them some more if you wish!

@MikeRalphson
Copy link
Contributor

Fab, thanks again!

@MikeRalphson MikeRalphson merged commit a57d4fe into Mermade:master Nov 19, 2018
@MikeRalphson
Copy link
Contributor

Included in shins@2.3.2-2

@matias-sandell matias-sandell deleted the included-files-pr branch November 20, 2018 07:12
@matias-sandell
Copy link
Contributor Author

matias-sandell commented Jan 24, 2019

@MikeRalphson, I now realise that this fix didn't actually help in cases where shins is included in a separate project from where the documentation is generated.

/ currently means that the files are included relative to the root shins path instead of source/includes, but that will resolve to /projects/myproject/node_modules/shins/myfolder/file-a.md if the user is using shins from a project folder called myproject.

A more likely case is that the user wants to include files from /projects/myproject/myfolder/file-a.md.

Do you want me to change the functionality of / from
includePath = path.join(__dirname, include + '.md');
to
includePath = path.join(process.cwd(), include + '.md');

Or should i submit a PR where the user can use // to achieve that, keeping the current functionality intact?

@MikeRalphson
Copy link
Contributor

I think the former (__dirname) option would be the most expected, thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants