FileSignatures icon indicating copy to clipboard operation
FileSignatures copied to clipboard

Extensions with period

Open asherber opened this issue 6 years ago • 5 comments

Thanks for this useful library!

I know this would be a breaking change, but I'd like to suggest that the values in FileFormat.Extension should start with a period, just like the output of Path.GetExtension() -- and like the table in this repo's README.

asherber avatar Dec 04 '19 22:12 asherber

I'm in two minds about this one. It probably does make sense to align with the way that Path.GetExtension works, but is that a good enough reason to create a breaking change?

I'm tempted to leave this issue open for a little while to give a chance to hear some more opinons on this.

neilharvey avatar Dec 06 '19 10:12 neilharvey

I totally understand. Maybe this is something you just want to hang onto until the next major release, rather than making a major release just for this.

Two other possibilities would be a static property, something like FileFormatInspector.ExtensionHasDot, which instructs file formats to include a dot before the extension (defaults to false) or an additional optional extensionHasDot parameter to the FileFormatInspector constructor which accomplishes the same thing.

I'm happy to work on a PR if you like one of those approaches.

asherber avatar Dec 06 '19 14:12 asherber

Totally useless comment: I'm ambivalent, either way. As long as I know how it works in any given version, I'm fine with the change. Might be nice to match Path.GetExtension, if for no other reason than maybe it has a history and therefore has set the precedent?

tiesont avatar Dec 06 '19 17:12 tiesont

@tiesont It's useful to hear that it wouldn't break what your use case, so thanks for the feedback :)

I've thought about this a bit over the weekend, and I'm inclined to change it. Having consistency with the standard libraries would help to make the usage more predicable, so that's probably a good enough reason to change it.

I also had a look at some of the other System.IO APIs, FileInfo.Extension works the same way, and Path.ChangeExtension produces the same regardless of whether the input which helped to convince me.

@asherber Thanks for the suggestion, but to make that work I'd have to make the formats mutable (either at the static or instance level) to make the option work which doesn't really appeal - I prefer having the formats fixed as immutable types (because a format doesn't change). I think what I'd prefer to do is just accept the breaking change and include the period in the definitions of each format.

neilharvey avatar Dec 09 '19 21:12 neilharvey