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

Pure POSIX sh version #12

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Pure POSIX sh version #12

wants to merge 20 commits into from

Conversation

WhitePeter
Copy link
Collaborator

@WhitePeter WhitePeter commented Apr 16, 2024

This is an attempt at getting rid of all bashisms to make mkvrg more portable by being pure POSIX sh. While I was looking at the script after being AWOL for a long time, I felt that some refactoring was in order to reduce clutter. I also don't like to rely on bash too much, as a matter of taste an style.

A further goal, which might warrant another branch, is to actually get rid of the handling of more than one file, because I find it unnecessary. I think it would be better to simplify the script and leverage OS utilities to do the "bigger picture" stuff. For instance there is GNU parallel, with which one can get multi-process execution, whereas this script is strictly sequential and can only do one track of one file at a time. Plus, bash and zsh have very powerful file name glob expressions, which makes the "recursive" mode superfluous.

Let me say it with this example, which uses zsh and parallel:

parallel -k mkvrg ::: **/*.mkv

For the uninitiated, ** (which also works in bash, as I have been told) is a convenient shortcut to similar functionality of find:

# same as above only with find
parallel -k mkvrg ::: $(find -iname '*.mkv')

I really think less is more, given the power of the Linux/Unix envrionment. "Do one thing and do it well", and glue together robust one-trick ponies to achieve your goal.

So, what do you think?

P.S.: Case in point, the handling of multiple files is the one thing that stands in the way of being pure POSIX sh, since it relies on arrays, which are an extension beyond POSIX.

Also add a vi modeline for convenience.
Don't need to assign a variable only to then unset it. The description
only get shown, if there is no further action anyway.
Added convenience functions to reduce clutter. Also got rid of some
bashisms with the long term goal of making this more portable by being
pure POSIX sh code.
According to the docs io-redirections should be done for the whole
compound command.
When in non-interactive mode, i.e. in a script, aliases are not
expanded. Aliases like `alias rm='rm -i'` are the primary reason for the
inflationary, and ill-advised, use of `rm -f`, which has more
implications than "overriding" `-i`. It basically does what it
advertises and, in the process, shuts up any warning that might be
useful in some corner cases.
This should now be a truly POSIX-compliant sh-script. Yay!

In the process, progress display via filePos() got a little improvement.
And just to prove a point I removed the `LC_NUMERIC=C` line, which
should not have been necessary in the first place. But I guess the bash
devs have a different opinion on that.
@WhitePeter WhitePeter marked this pull request as ready for review April 18, 2024 07:12
@WhitePeter WhitePeter changed the title Less bash Pure POSIX sh version Apr 18, 2024
Use printf only when it is actually needed, i.e. when generating the
tags. Also use `grep -E` rather than `-P` since this is about POSIX
compliance.

P.S.: I take back what I said about LC_NUMERIC.
Totally forgot to set IFS.
Nice to have folding. Plus, it adds some rudimentary code documentation.
There were a lot of stale tmp files in my $TMPDIR, because cleantmp()
did not always run, i.e. when exiting without calling it explicitly.
Explicit call of cleantmp removed.
Nice to know which signal caused the exit. Before, there was an error
stating that `$tmpFile` could not be removed when in fact it was. Took
me a while to realize that the trap for EXIT had to be reset.
New function tag_tracks() gets called right after successful remux.

Fixes #16
I think it is more convenient to set options as arguments to the program
rather than by setting environment variables. The latter is still
possible but command line options will take precedence in case of
conflict. No documentation yet.
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.

1 participant