use path.extname instead of regex comparisons

[msimerson]

In configfile, we have an if/else chain that inspects a filename to determine its type. Instead, we should likely use path.extname.

[ghost]

Are we talking about this if block?

haraka-config/configfile.js

Lines 339 to 353 in e82a402

	if (!fs.existsSync(name)) {
	if (!/\.h?json$/.test(name)) {
	return cfrType.empty(options, type);
	}
	const yaml_name = name.replace(/\.h?json$/, '.yaml');
	if (!fs.existsSync(yaml_name)) {
	return cfrType.empty(options, type);
	}
	name = yaml_name;
	type = 'yaml';
	cfrType = cfreader.get_filetype_reader(type);
	}

[msimerson]

Bah I wrote the wrong filename. It's here in config.js

[msimerson]

The general ideas being, make that one less place to require code maintenance when we add support for some future file type.

[ghost]

Oh, ok I will fix it.

[ghost]

If this is the purpose should I do something about this line too?

haraka-config/config.js

Line 120 in e82a402

if (/^(ini|value|list|data|h?json|yaml|binary)$/.test(args[i])) {

[msimerson]

Hrmm, a string found there would be the user explicitly specifying the file content type. As you can see, some of the types (value, list, data) don't map to filename extensions. I don't know what you can do there that would improve it.

[ghost]

Ok from this:

haraka-config/config.js

Lines 131 to 137 in e82a402

	if (!fs_type) {
	if (/\.hjson$/.test(fs_name)) fs_type = 'hjson';
	else if (/\.json$/.test(fs_name)) fs_type = 'json';
	else if (/\.yaml$/.test(fs_name)) fs_type = 'yaml';
	else if (/\.ini$/.test(fs_name)) fs_type = 'ini';
	else fs_type = 'value';
	}

I boiled it down to this:

if (!fs_type) {
    var fs_ext = path.extname(fs_name);

    switch (fs_ext) {
        case ".hjson":
        case ".json":
        case ".yaml":
        case ".ini":
            fs_type = fs_ext.substring(1);
            break;

        default:
            fs_type = 'value';
            break;
    }
}

Opinion?

[msimerson]

Almost perfect. You'll fail lint tests because of the var. Use const instead. The other thing I'd do differently (not required) is consistently use single quotes. They're easier to read and I'm in the habit of always using ' unless I "want" interpolation.

[baudehlo]

Perl habits die hard, eh?

…

On Fri, Sep 29, 2017 at 3:39 PM, Matt Simerson ***@***.***> wrote: Almost perfect. You'll fail lint tests because of the var. Use const instead. The other thing I'd do differently (not required) is consistently use single quotes. They're easier to read and I'm in the habit of always using ' unless I "*want*" interpolation. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#31 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAobY3wO--vp6zNb7b3n-tE8VB6_zejCks5snUdogaJpZM4PnvHL> .

[ghost]

Ok, got it. I tend to use double quotes because they mean string where single quote usually means single character. But I will abide.

[msimerson]

Perl habits die hard, eh?

LOL. Actually, sh/bash/POSIX, which predate my perl habits and which I still use very frequently.

[msimerson]

where single quote usually means single character

Interesting, in what context?

[ghost]

Mainly C# and like. I am not sure but I think C and C++ also do this.

Edit: I was correct, https://stackoverflow.com/a/3683613/1829884.

[msimerson]

Darn you! I've tried so hard to forget the C I nearly learned so long ago, and now you're running my nose in it. ;-)

[ghost]

I did one small change please take a look before I issue a PR. https://github.com/PSSGCSim/haraka-config/commit/bdaaa02a7e6db410151922d054251bd4c1304fa7 It is regarding where the substring (line 132) occurs.

[msimerson]

Even better, getting rid of the redundant leading dots. 👍