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

Add --output-dir parameters to commands 'serve' and 'build' #191

Closed
wants to merge 1 commit into
base: next
from

Conversation

Projects
None yet
2 participants
@verpeteren
Contributor

verpeteren commented Dec 4, 2017

This is useful for example when you want to generate a site to a different document-root.

@Keats

This comment has been minimized.

Collaborator

Keats commented Dec 4, 2017

This is useful for example when you want to generate a site to a different document-root.

What are the reasons against using the default one?

@verpeteren

This comment has been minimized.

Contributor

verpeteren commented Dec 4, 2017

I'd like to create several themes that can work together with the same content.
Currently, I use gutenberg build && mv public var/www/something.

It would be convienient to render different variants by just changing the theme = something in the config.toml file, gutenberg --base-url https://example/com/something --output-dir var/www/something.

(Ideally, an additional '--config-file config_something.toml' parameter, would make this perfect.)

@Keats

Few comments but looks good otherwise.
Can you change the PR to be on the next branch though please?

@@ -5,8 +5,9 @@ use site::Site;

use console;

pub fn build(config_file: &str, base_url: Option<&str>) -> Result<()> {
pub fn build(config_file: &str, base_url: Option<&str>, public: &str) -> Result<()> {

This comment has been minimized.

@Keats

Keats Dec 5, 2017

Collaborator

I would name public output_dir instead, public is not very explicit in the code

@@ -77,7 +77,7 @@ fn rebuild_done_handling(broadcaster: &Sender, res: Result<()>, reload_path: &st
}
}

pub fn serve(interface: &str, port: &str, config_file: &str) -> Result<()> {
pub fn serve(interface: &str, port: &str, public: &str, config_file: &str) -> Result<()> {

This comment has been minimized.

@Keats

Keats Dec 5, 2017

Collaborator

same as above

src/main.rs Outdated
@@ -43,7 +43,8 @@ fn main() {
("build", Some(matches)) => {
console::info("Building site...");
let start = Instant::now();
match cmd::build(config_file, matches.value_of("base_url")) {
let public = matches.value_of("output_dir").unwrap_or("public");

This comment has been minimized.

@Keats

Keats Dec 5, 2017

Collaborator

Is the unwrap needed since there is a default value in clap?

src/main.rs Outdated
@@ -54,8 +55,9 @@ fn main() {
("serve", Some(matches)) => {
let interface = matches.value_of("interface").unwrap_or("127.0.0.1");
let port = matches.value_of("port").unwrap_or("1111");
let public = matches.value_of("output_dir").unwrap_or("public");

This comment has been minimized.

@Keats

Keats Dec 5, 2017

Collaborator

same as above

This comment has been minimized.

@Keats

Keats Dec 6, 2017

Collaborator

I'm just not sure how the default works, I guess you can try removing the unwrap_or and see if it still works?

This comment has been minimized.

@verpeteren

verpeteren Dec 7, 2017

Contributor

Without the unwrap_or, the type goes from &str to Option<&str>.
It could be made working with if let Some(x) = ..{}, but IMHO that would be less clearer then the unwrap_or.

What might be interesting, to make the main.rs more uniform, is make the same unwrap_or for base_url change:

Please advice.

This comment has been minimized.

@Keats

Keats Dec 7, 2017

Collaborator

the type goes from &str to Option<&str>

I guess with a default, it can never be None right? So a normal .unwrap() would work

$ gutenberg build --output-dir $DOCUMENT_ROOT
```

This is useful for example when you want to generate a site to a different document-root.

This comment has been minimized.

@Keats

Keats Dec 5, 2017

Collaborator

This sentence seems a bit redundant, I would remove it

verpeteren added a commit to verpeteren/gutenberg that referenced this pull request Dec 11, 2017

@verpeteren verpeteren changed the base branch from master to next Dec 11, 2017

@Keats

This comment has been minimized.

Collaborator

Keats commented Dec 12, 2017

Looks like tests are failing now. Can you also remove my commit from your branch?

@Keats

This comment has been minimized.

Collaborator

Keats commented Dec 13, 2017

There's still my commit in the list. You can probably just merge all your commits and delete mine to fix it

@verpeteren

This comment has been minimized.

Contributor

verpeteren commented Dec 15, 2017

Sorry; my git knowledge is not sufficient.
That is why I manually reverted your commit.
Could you guide me through the needed steps?

@Keats

This comment has been minimized.

Collaborator

Keats commented Dec 15, 2017

So that would look something like:

$ git rebase -i HEAD~7

(7 here is a number representing all the commits in the branch, the one at the top should be mine)

On my commit and the manual revert replace pick with d. On all other commits except your first one, replace pick with s. That should leave you with a single commit with all the changes that you can git push -f.
Another approach but which would require a new PR I believe would be to do git rebase -i HEAD~6, drop the manual merge in the interface and use s for all commits except the first one and copy that resulting commit hash.
On a new branch based on next, you can git cherry-pick myhash to move the commit onto that new branch.

Merge pull request #181 from tvincent2/add-changekind-sass-test
Add the detection of ChangeKind::Sass to tests

Add --output-dir parameters to commands 'serve' and 'build'

Update documentation and command line completion with 'output-dir' flag

processed feedback

See #191 for details.

Add trailing_slash opt. to get_url (#173)

* Added inital trailing_slash impl

* Added simple test

* Updated docs website to use trailing_slash option

* Updated documentation to reflect new trailing_slash option

* Added combined cachebust and trailing_slash test

Add get_taxonomy_url global_fn

And fix bug with taxonomies urls

Add earliest Rust version to travis

Fix tests and add taxonomies to changelog

Update to Tera 0.11 beta

Update Tera in cargo.lock

Add default templates

repeat paste fix

@verpeteren verpeteren force-pushed the verpeteren:output_path branch from 44305fe to fbecd9e Dec 22, 2017

@Keats

This comment has been minimized.

Collaborator

Keats commented Dec 29, 2017

Ha I think the rebase merging got some earlier commits. Git 😨
I'll add the changes locally myself and push to the branch directly

Keats added a commit that referenced this pull request Dec 29, 2017

Keats added a commit that referenced this pull request Dec 29, 2017

@Keats

This comment has been minimized.

Collaborator

Keats commented Dec 29, 2017

I've added the changes in 14edd2b
Thanks!

@Keats Keats closed this Dec 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment