Davejkane / riv

Riv - The Rust Image Viewer
MIT License
99 stars 10 forks source link

Shell independent glob expansion #50

Closed gurgalex closed 5 years ago

gurgalex commented 5 years ago

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.

NickHackman commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

OK, cool.

gurgalex commented 5 years ago

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