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

Fix error message mentionning graphicsmagick instead of imagemagick #19

Merged
merged 1 commit into from
Jun 24, 2019
Merged

Fix error message mentionning graphicsmagick instead of imagemagick #19

merged 1 commit into from
Jun 24, 2019

Conversation

nicoulaj
Copy link
Contributor

The error message mentions graphicsmagick, but the program actually calls convert which is provided by imagemagick.
You can have graphicsmagick "installed as imagemagick" (eg: https://aur.archlinux.org/packages/graphicsmagick-imagemagick-compat/), but I think it would be less confusing just mentionning imagemagick.

@phiresky
Copy link
Owner

oh. yeah that was because I used to have it use graphicsmagick (cuz much faster usually) but then I had some problem with that. Thanks!

@phiresky phiresky merged commit b75b8ee into phiresky:master Jun 24, 2019
@giovannibonetti
Copy link

On the other hand, when running the command like this...

rga something -g '*.pdf' --rga-adapters=+pdfpages,tesseract

...I get a warning that says gm is required:

-------------------------------------------------------------------------------
Warning: found multiple adapters for my-pdf-file.pdf:
 - pdfpages
 - poppler
adapter: pdfpages
Error: Could not find executable "gm". Make sure you have imagemagick installed.
-------------------------------------------------------------------------------

After installing graphicsmagick the warning disappeared and everything worked fine.
Running on macOs Catalina v10.15.4

@phiresky
Copy link
Owner

Huh, now I'm confused. Looks like it does use gm after all:

        let exe_name = "gm";
        let mut cmd = Command::new(exe_name);
        cmd.arg("convert")
            .arg("-density")
            .arg("200")
            .arg(inp_fname)
            .arg("+adjoin")
            .arg(out_fname);

Maybe @nicoulaj misread the code (like I did when looking again before merging the PR)? But then why did they submit this at all, did it show some different issue without having imagemagick installed?

cpick pushed a commit to cpick/ripgrep-all that referenced this pull request Dec 2, 2020
…magick"

As mentioned in
[a followup comment](phiresky#19 (comment))
when this change from "graphicsmagick" to "imagemagick" was originally
merged, the code does currently call `gm` (src/adapters/pdfpages.rs:62)
and so does, in fact, need the "graphicsmagick" package.

This reverts commit 6d2df91.
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

3 participants