fmang/opustags

Why treat empty filenames specially?

rrthomas opened this issue · 5 comments

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

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.

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

I got that point, but look at:

opustags/src/cli.cc

Lines 73 to 74 in 6f7ac1f

if (!opt.path_out.empty())
return {st::bad_arguments, "Cannot specify --output more than once."};

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.

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.

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.