fmang / opustags

Ogg Opus tags editor
BSD 3-Clause "New" or "Revised" License
81 stars 11 forks source link

Why treat empty filenames specially? #27

Closed rrthomas closed 4 years ago

rrthomas commented 4 years ago

opustags complains if a filename argument is empty. Most programs don't do this, they will just give an error when they try to open a file with an empty name.

fmang commented 4 years ago

You’re absolutely right. That applies to both input and output file. We’ll need an extra state to differentiate empty path_out and unspecied path_out when validating options.

rrthomas commented 4 years ago

Sorry, it seems I wasn't clear: my suggestion was that we don't need any special code to deal with empty path_out, and we don't validate that path_out is non-empty. If an empty string is supplied as path_out (or indeed path_in), then a file error will result when opustags tries to open the given file, just as with any other invalid file name, or a name that fails for some other reason (e.g. it refers to a non-existent file, or a filing system object that is not a file).

fmang commented 4 years ago

I got that point, but look at: https://github.com/fmang/opustags/blob/6f7ac1f13bbbe30b173bc4c7ea2d52da2051bea7/src/cli.cc#L73-L74

If we accept empty as a valid path_out, then this test breaks because we won’t be able to distinguish the case -o "" (path_out is empty because the user specified an empty value) and the case with no -o at all (path_out empty by default). If we just dropped the validation, then the CLI won’t detect any error with -o "" -o x.

rrthomas commented 4 years ago

This is no problem. Just as in any other case, if a valid filename overrides an invalid one, there's no reason why that should be detected. We don't validate filenames before using them. Again, this is how standard command-line utilities behave.

rrthomas commented 4 years ago

I realize that you're also referring to the fact that you can't specify --output more than once. Again, we can detect that properly, by detecting whether the string has been set, not whether it's empty.