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

feat: add support for recursively scanning directories #49

Merged
merged 5 commits into from
Jan 9, 2022

Conversation

JakeStanger
Copy link
Collaborator

@JakeStanger JakeStanger commented Jan 8, 2022

This works towards implementing features discussed in #3. It is still using the config.json format at the moment. I'll pick up discussion in #28 for migrating to a human-friendly format.

You can now eg navigate to ~/Programming and run dura watch to watch the whole directory recursively. The program will scan each child dir until either a repo is found or the max depth is hit.

Includes, excludes and the max depth can be manually tweaked in the config.json file.

If include is non-empty, then exclude is ignored.

Example config which includes two repos, one at Programming/dura and the other at Programming/unity/my-game:

{"pid":27814,"repos":{"/home/jake/Programming":{"include":["dura", "unity/my-game"],"exclude":[],"max_depth":255}}}

Example config which excludes all repos inside Programming/third-party:

{"pid":27814,"repos":{"/home/jake/Programming":{"include":[],"exclude":["third-party"],"max_depth":255}}}

This means you can now eg navigate to `~/Programming` and run `dura watch` to watch the whole directory recursively. The program will scan each child dir until either a repo is found or the max depth is hit.

Includes, excludes and the max depth can be manually tweaked in the `config.json` file.
Copy link
Owner

@tkellogg tkellogg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change the include/exclude algorithm? Other than that, if you're feeling ambitious, try to create a test for this in end_to_end_test.rs. I'm curious how easy it is for others to write tests in that file.

src/poller.rs Outdated Show resolved Hide resolved
src/poller.rs Outdated Show resolved Hide resolved
src/poller.rs Outdated Show resolved Hide resolved
let latency = (Instant::now() - start_time).as_secs_f32();
let repo = current_path
.to_str()
.unwrap_or("<invalid path>")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't terribly like using a null sentinel value here, seems like evades the type system.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think cargo format has authored that to me due to a line-break but it was already there. Happy to make changes while I'm here though. Not sure how to best handle this specific scenario as it's quite an edge case - any preferences?

src/poller.rs Show resolved Hide resolved
src/poller.rs Outdated Show resolved Hide resolved
src/poller.rs Outdated Show resolved Hide resolved
src/poller.rs Outdated Show resolved Hide resolved
Copy link
Owner

@tkellogg tkellogg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

waiting for the include/exclude algorithm fix

@JakeStanger
Copy link
Collaborator Author

Going to make the changes to the algorithm in a sec. Once that's done I'll push a commit that should resolve everything outstanding.

if you're feeling ambitious, try to create a test for this in end_to_end_test.rs

More than happy to write a test, but I'm struggling a little bit as to exactly what I should be testing.

@tkellogg
Copy link
Owner

tkellogg commented Jan 8, 2022

@JakeStanger something along the lines of

  1. create directory via GitRepo::init
  2. create a few directories
  3. write a config file via the Dura struct
  4. make a change & do dura capture
  5. check last commit to see if it included the right files

Algorithm now works as follows:

1. Include everything in dir
2. Exclude anything in the exclude list.
3. Include everything from \1 that's also in include list, even if it were excluded in 2

The code has been tidied up in several places to improve error handling and quality.
@JakeStanger
Copy link
Collaborator Author

I've just pushed an unfinished/broken test that's a good starting point, as I've gotta drop off and might not be able to get it finished for a few days.

If somebody's able to pick it up and get it working that'd be awesome, otherwise if you want me to drop the commit for now and merge it without let me know & I can try and do that tomorrow.

@tkellogg
Copy link
Owner

tkellogg commented Jan 9, 2022

i took a crack at the test but ran out of time. it's harder than i anticipated. i'll see if i can make it easier to test next week. let's just rollback the test and merge it

@JakeStanger
Copy link
Collaborator Author

Cool, I've got ~30 mins so let me know if you want anything else done now

@tkellogg tkellogg merged commit 31a605a into tkellogg:master Jan 9, 2022
@JakeStanger JakeStanger deleted the feat/recurse branch January 10, 2022 20:24
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

2 participants