Watch include exclude #94

malobre · 2020-11-28T19:01:53Z

This adds the --watch <path>... option to the serve and watch subcommands which allows to watch specific folder(s) or file(s).

Closes #93

Checklist

Updated CHANGELOG.md describing pertinent changes.
~~Updated README.md with pertinent info (may not always apply).~~

malobre · 2020-11-28T19:09:14Z

I've replaced the --ignore argument with --exclude (because the other new arg is --include).
It's a breaking change, how do you feel about that @thedodd ?

thedodd · 2020-11-30T15:46:03Z

@malobre awesome! Thanks for tackling this. I would actually be in favor of preserving the current --ignore arg, and then we change the name of the new arg to --watch instead of --include. Thoughts? Primarily I just don't want to release a breaking change when it is unnecessary to do so. I'm flexible on the name of the new argument though.

malobre · 2020-11-30T16:08:48Z

--watch feels weird:

$ trunk watch --watch folder

How about:

$ trunk watch --path some/folder/src --ignore some/folder/src/readme.md

thedodd · 2020-11-30T16:27:51Z

@malobre perhaps. However, that is how cargo-watch does it. It may feel familiar to some.

Also, trunk watch isn't the only command which will be used for this. Honestly, it will probably be far more often that folks will be using trunk serve --watch pathx with this. Or, in both cases, may likely have this stuff in their Trunk.toml.

Thoughts?

malobre · 2020-11-30T16:35:26Z

That's a valid point, I do not feel strongly about one or the other. I'll name them however you like :-)

thedodd · 2020-11-30T16:46:20Z

I dig it. Let's do --watch then, in hopes that it will feel familiar for folks that have used cargo watch before, and in hopes that it will have an intuitive feel when used with trunk serve and such.

@malobre thanks for the discussion! Ping me when this guy is ready for review. Thanks again for the work on this.

malobre · 2020-11-30T17:10:47Z

I think most of the work is done, I still need to do a little more testing though.

I've only tested this:

$ trunk watch examples/yew/index.html --watch examples/yew --ignore examples/yew/README.md

Which seems to work !

malobre · 2020-11-30T17:23:15Z

Tested:

No --watch, (should default to target build directory)
--watch some/directory
--watch some/file.txt
--watch first/folder --watch second/folder
--watch some/directory --ignore some/directory/file.txt

malobre · 2020-11-30T17:24:43Z

@thedodd Ready for review !

malobre · 2020-12-02T11:01:18Z

I just pushed a small change that enhance the help messages:

-i, --ignore <ignore>...         Paths to ignore [default: []]

becomes

-i, --ignore <path>...           Paths to ignore [default: []]

The same goes for --watch.

thedodd · 2020-12-03T17:16:42Z

@malobre woot woot! Very nice! Two things that I will ask real quick before I review (I'll add these to the PR template as well):

looks like there are 11 commits on this PR. I would just ask that you squash those down to 1 or 2 logical commits which describe your work in a clear and concise way.
maybe as part of that squash, you can address the one CI lint issue as well :)

Thanks again for all this work! I should be able to get this reviewed, merged & released over the next few days.

thedodd

A couple of small changes. All in all, this is top-notch! Thanks @malobre!

Trunk.toml

src/config/models.rs

src/config/rt.rs

src/watch.rs

malobre · 2020-12-03T22:06:24Z

Woops looks like I messed up my rebase a little bit. I'll fix that.

The new option allows to watch specific folder(s) or file(s) when using the `serve` or `watch` subcommand.

src/watch.rs

thedodd · 2021-01-23T18:43:37Z

@malobre hey boss. Just following-up with you here. Looks like linting is failing for various reasons. I have a PR I'm about to merge which will hopefully address some of that stuff. I'll update this branch after that.

Once it is updated, if there are still clippy issues, then it means that they are unique to your PR :) so I'll ask you to fix them if that is the case. Cheers!

malobre · 2021-01-24T09:21:15Z

Okay, no problem. Ping me when needed.

thedodd

+1 excellent work! Sorry for the amount of time it took for me to do the final review on this and get it merged!

thedodd · 2021-01-25T04:39:06Z

src/config/rt.rs

+    /// Paths to watch, defaults to the build target parent directory.
+    pub paths: Vec<PathBuf>,
+    /// Paths to ignore.
+    pub ignored_paths: Vec<PathBuf>,
 }

 impl RtcWatch {
    pub(super) fn new(build_opts: ConfigOptsBuild, opts: ConfigOptsWatch) -> Result<Self> {
        let build = Arc::new(RtcBuild::new(build_opts)?);
+
+        let paths = {
+            let mut paths = opts.watch.unwrap_or_default();
+
+            if paths.is_empty() {
+                paths.push(
+                    build
+                        .target
+                        .parent()
+                        .ok_or_else(|| anyhow!("couldn't get parent of {:?}", build.target))?
+                        .to_path_buf(),
+                )
+            }
+
+            paths
+        };


Looks like this will not include the target dir's parent dir if it is not explicitly declared. That's fine. Just adding a note here. After I merge this, I'll update the docs to state that info.

malobre · 2021-01-25T09:33:09Z

No worries, we all have stuff going on in our lives. :)

thedodd · 2021-02-01T02:29:43Z

@malobre just wanted to give you a heads-up that we will be releasing this shortly! Should land in the 0.8.0 release.

malobre mentioned this pull request Nov 28, 2020

Set directories to watch other than the current directory #93

Closed

malobre marked this pull request as ready for review November 30, 2020 17:23

thedodd requested changes Dec 3, 2020

View reviewed changes

Trunk.toml Outdated Show resolved Hide resolved

src/config/models.rs Show resolved Hide resolved

src/config/rt.rs Outdated Show resolved Hide resolved

src/watch.rs Outdated Show resolved Hide resolved

malobre added 2 commits December 3, 2020 23:38

Add --watch option to serve and watch.

8124ca7

The new option allows to watch specific folder(s) or file(s) when using the `serve` or `watch` subcommand.

Allocate vec with capacity to prevent reallocation

f30f31e

malobre commented Dec 3, 2020

View reviewed changes

src/watch.rs Show resolved Hide resolved

malobre requested a review from thedodd December 3, 2020 22:50

malobre mentioned this pull request Dec 13, 2020

Fix infinite Watch Loop (Related to PR #84) #99

Closed

1 task

Merge branch 'master' into watch_include_exclude

0b769c9

thedodd approved these changes Jan 25, 2021

View reviewed changes

thedodd merged commit 670ccf9 into trunk-rs:master Jan 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Watch include exclude #94

Watch include exclude #94

malobre commented Nov 28, 2020 •

edited

Loading

malobre commented Nov 28, 2020

thedodd commented Nov 30, 2020

malobre commented Nov 30, 2020

thedodd commented Nov 30, 2020

malobre commented Nov 30, 2020

thedodd commented Nov 30, 2020

malobre commented Nov 30, 2020

malobre commented Nov 30, 2020 •

edited

Loading

malobre commented Nov 30, 2020

malobre commented Dec 2, 2020

thedodd commented Dec 3, 2020

thedodd left a comment

malobre commented Dec 3, 2020 •

edited

Loading

thedodd commented Jan 23, 2021

malobre commented Jan 24, 2021

thedodd left a comment

thedodd Jan 25, 2021

malobre commented Jan 25, 2021

thedodd commented Feb 1, 2021

Watch include exclude #94

Watch include exclude #94

Conversation

malobre commented Nov 28, 2020 • edited Loading

malobre commented Nov 28, 2020

thedodd commented Nov 30, 2020

malobre commented Nov 30, 2020

thedodd commented Nov 30, 2020

malobre commented Nov 30, 2020

thedodd commented Nov 30, 2020

malobre commented Nov 30, 2020

malobre commented Nov 30, 2020 • edited Loading

malobre commented Nov 30, 2020

malobre commented Dec 2, 2020

thedodd commented Dec 3, 2020

thedodd left a comment

Choose a reason for hiding this comment

malobre commented Dec 3, 2020 • edited Loading

thedodd commented Jan 23, 2021

malobre commented Jan 24, 2021

thedodd left a comment

Choose a reason for hiding this comment

thedodd Jan 25, 2021

Choose a reason for hiding this comment

malobre commented Jan 25, 2021

thedodd commented Feb 1, 2021

malobre commented Nov 28, 2020 •

edited

Loading

malobre commented Nov 30, 2020 •

edited

Loading

malobre commented Dec 3, 2020 •

edited

Loading