Skip to content
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

Allow setting includes directory and non-absolute copy #111

Merged
merged 5 commits into from
Jun 22, 2021

Conversation

fserb
Copy link
Contributor

@fserb fserb commented Jun 22, 2021

This PR adds a includes option to change the _includes directory and allows site.copy() to copy from directories relative to src (dest is still absolute).

The rationale behind this is: for one of my projects, I have loads of top level files, I want to keep them in a subdirectory. When I change src, I'm kinda forced to change my assets and _include, because they "follow" the src. With this change, I'm able to specify includes: "../_includes" and site.copy('../assets', 'assets').

Alternatively, we could make those two (includes and copy) to be relative to the execution, but I'm not sure that's a good idea, because it breaks the "I want to isolate a build" use case.

@oscarotero
Copy link
Member

About the option to configure the _includes directory, It's a good idea. But keep in mind that this value is used in several engines and plugins, so you need to change in all places. Take a look here: https://github.com/lumeland/lume/search?q=_includes&type=code

And about the copy() change, I'm not sure to understand. You can already do site.copy('../assets', 'assets').

@fserb
Copy link
Contributor Author

fserb commented Jun 22, 2021

Updated all _includes, sorry for that.

Regarding copy(). I don't think that's true, because join('/', '../x') == '/x', I mean, there's even a test for that. I've replaced with join('./', from) for now.

That said, this is not correct yet, the watch on serve stopped working on assets once I did that. Looking into that now.

@fserb
Copy link
Contributor Author

fserb commented Jun 22, 2021

Found it.

@fserb
Copy link
Contributor Author

fserb commented Jun 22, 2021

Fixed it :)

@fserb
Copy link
Contributor Author

fserb commented Jun 22, 2021

argh. The copy is still not working properly. Now it detects the change, but it doesn't copy the correct files on change.

@fserb
Copy link
Contributor Author

fserb commented Jun 22, 2021

Fixed. :)

@oscarotero
Copy link
Member

I'm not absolutely convinced about copy, but I'll merge and try it.

@oscarotero oscarotero merged commit 0f67cdc into lumeland:master Jun 22, 2021
@fserb fserb deleted the includes branch June 22, 2021 18:29
@oscarotero
Copy link
Member

oscarotero commented Jun 22, 2021

I've testing the copy()change and it generates some issues. For example, on read the file to embed in the html by the inline plugin. I'm not confortable with letting to load and process files outside src folder. It looks a bit insecure and inconsistent (for example, the watcher now have to watch all these outside paths, and you forget to add also the _includes folder).

Sorry but I'm going to revert this change. Some ideas:

  • You can use a script to copy manually the files and folder outside the src folder. For example:
    site.addEventListener("beforeBuild", "cp ../outside-file.txt _dest/new-file.txt");
  • I can include an option to add, manually more paths to the watcher:
    site.watch("../outside-file.txt");

What do you think?

@fserb
Copy link
Contributor Author

fserb commented Jun 22, 2021

Hmmmmmmmmm. Okey. What do you think about doing the opposite then?

We keep src as "the root" of the project and keep all the join('/', xx) with the guarantee that we won't touch any file outside it.

And we add a path to be the "page dir" (that by default is ".") that decides where we are going to look for pages and what is the base path (on dest) for them?

@oscarotero
Copy link
Member

Mmmm, that's could be complex too. I'm doing some testing with symlinks, so I think it's the most clean and easy way. Just create a symlink in your src folder pointing to external sources. Does it fit to you?

@fserb
Copy link
Contributor Author

fserb commented Jun 22, 2021

Yes, sure, symlink solves it.

Maybe I could also solve this by either setting a URL data function that removes the top level dir or by changing the URLs on beforeBuild/beforeUpdate?

@shah
Copy link

shah commented Jun 22, 2021

FWIW, I agree with creating "helper" functions as scripts to do anything complex outside of src directory and symlinks are ideal ways of bridging any gaps.

@oscarotero
Copy link
Member

If the problem is just the url, the most easy way is creating a url function in a _data.js file to change the default url of all pages. For example:

export function url(page) {
    let url = page.dest.path + page.dest.url;
    return url.replace("/basepath", "");
} 

See https://lumeland.github.io/creating-pages/urls/

@fserb
Copy link
Contributor Author

fserb commented Jun 22, 2021

Just came here to write this. :)

But I did

export function url(page) {
  page.dest.path = page.dest.path.replace(/^\/basepath\//, '/');
}

instead. So it still uses prettyUrls.

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

Successfully merging this pull request may close these issues.

None yet

3 participants