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

DGIndexNV: list of files throws error #27

Closed
LightArrowsEXE opened this issue Sep 9, 2023 · 2 comments
Closed

DGIndexNV: list of files throws error #27

LightArrowsEXE opened this issue Sep 9, 2023 · 2 comments

Comments

@LightArrowsEXE
Copy link
Member

This error seems to happen if there are any spaces in the path.
stdout doesn't return any useful errors.

Even changing this in /indexers/DGIndexNV.py:L27 doesn't work:

-','.join(str(path) for path in files),
+','.join('\"{path}\"' for path in files),

I suspect this same error will also happen with the -o file.
Since the above code doesn't work for me, I'm not sure what else would.

@LightArrowsEXE
Copy link
Member Author

LightArrowsEXE commented Sep 9, 2023

Doing this fixed it, but you can probably figure out a better way to write this:

/indexers/DGIndexNV.py:L23

    def get_cmd(self, files: list[SPath], output: SPath) -> list[str]:
        return list(
            map(str, [
                self._get_bin_path(), '-i',
                ','.join(path.name for path in files),
                '-h', '-o', f"{output.parent.name}\\{output.name}",
                '-e'
            ])
        )

/indexers/base.py:157

        proc = subprocess.Popen(
            list(map(str, self.get_cmd(files, output) + list(cmd_args) + list(self._default_args))),
            text=True, encoding='utf-8', shell=True, cwd=output.get_folder().parent.to_str()
        )

The idea is simple: Rather than deal with the path with spaces, we simply cd into the directory containing all the VOBs. There is one major downside to this, though: it breaks if you have source files in multiple directories. So this doesn't necessarily fix the issue itself, it just sidesteps it in the most common use-case where you'd pass multiple files.

Because this still fixes the most common use-case, I think it's worth adding some kind of check to see whether all the files are in the same directory or not and then cd into that directory to process the files, but again, this doesn't fix the root issue.

@Setsugennoao
Copy link
Member

Shouldn't be an issue with the changes made by @jsaowji

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

No branches or pull requests

2 participants