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

Shell independent glob expansion #50

Closed
gurgalex opened this issue May 15, 2019 · 9 comments · Fixed by #64
Closed

Shell independent glob expansion #50

gurgalex opened this issue May 15, 2019 · 9 comments · Fixed by #64
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@gurgalex
Copy link
Collaborator

From #49

Some shells may not have support for globstar ** expansion.

Bash has it disabled by default.

To support shells that don't have this feature a quoted string can be supplied (with a cli flag as well?).
riv --placeholder **/*.png

The program will do shell expansion instead of the shell.

Also may be of use for extremely large amounts of images. As the length of the expanded arguments may be larger than the OS supports.

@Davejkane Davejkane added enhancement New feature or request help wanted Extra attention is needed labels May 16, 2019
@Davejkane Davejkane added this to the 0.4 milestone May 16, 2019
@NickHackman
Copy link
Contributor

NickHackman commented May 19, 2019

I'm game to work on this since I need it for #40

I think implementing this solely in this application wouldn't be beneficial and that if we can't find another library that performs this operation, we might as well write a separate library to perform this because this would supersede the glob library that we're currently using.

@gurgalex
Copy link
Collaborator Author

gurgalex commented May 19, 2019

I was thinking shellexpand (for tilde expansion)
and then passing the glob string to glob::glob to process the string may work.

Cli argument paths and the flag for shell independent globs should be mutually exclusive (clap has support for this conflicts_with)
or each overrides one another overrides_with

@NickHackman
Copy link
Contributor

Uhh, my previous comment was a brain fart.

I have a pull request in shellexpand fixing the deprecated use of std::env::home_dir. Shell expand would allow us to also use $HOME/images.jpg too, so that would be nice!

@Davejkane
Copy link
Owner

How does shellexpand help with globbing? As a user I'd like to be able to type "~/**/*.png". Is shellexpand going to be able to help with that or are we going to have to separate the glob from the rest of the path ourselves?

@gurgalex
Copy link
Collaborator Author

It doesn't help with globbing.

It helps with expanding ~/*.png to /home/alex/*.png so that it can be passed to glob::glob for finding matching file paths.

@Davejkane
Copy link
Owner

Isn't there also a problem with the glob library not handling absolute paths correctly? I think it automatically inserts the current dir regardless, so /usr/local/*.png get's expanded to /path/to/current/dir//usr/local/*.png.

@gurgalex
Copy link
Collaborator Author

No, I think that was because in previous version of riv the current directory was always prepended to all glob strings.

The following code handles non-current directory globs just fine.

use glob;
use std::error::Error;
use std::path::PathBuf;

fn main() -> Result<(), Box<Error>> {
    let current_dir = PathBuf::from(".").canonicalize()?;
    println!("current_dir: {:?}\n", current_dir);
    let glob_for = "/home/alex/test_dir/**/*.png";

    println!("results for glob: {}", glob_for);
    let paths = glob::glob(glob_for)?;
    for path in paths {
        let result = path?;
        println!("{:?}", result);
    }
    Ok(())
}

Results

current_dir: "/home/alex/rust_projects/scratch"

results for glob: /home/alex/test_dir/**/*.png
"/home/alex/test_dir/a.png"
"/home/alex/test_dir/b.png"
"/home/alex/test_dir/subdir/c.png"

@Davejkane
Copy link
Owner

OK, cool.

@gurgalex
Copy link
Collaborator Author

gurgalex commented May 20, 2019

Removed context field. Which fixed ~, but also fixed absolute paths.
https://github.com/Davejkane/riv/pull/45/files#diff-10c5f11df40746fb61a07b0195c723e7L12

@Davejkane Davejkane self-assigned this May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants