Skip to content

feat: config support#562

Open
Pendonym wants to merge 3 commits intop2r3:masterfrom
Pendonym:master
Open

feat: config support#562
Pendonym wants to merge 3 commits intop2r3:masterfrom
Pendonym:master

Conversation

@Pendonym
Copy link

Add support for all the main configuration file formats


async init() {
this.supportedFormats = [
CommonFormats.JSON.builder("json").markLossless().allowFrom(true).allowTo(true),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think lossless applies here - JSON has some type information that plaintext configs don't.

Comment on lines +110 to +114
if (outputFiles.length === 0) {
throw new Error(
`configHandler does not support route: ${inputFormat.internal} -> ${outputFormat.internal}`,
);
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make sense to support config->config by simply renaming the file? I get that this handler assumes all text-based configs are ini anyways.

import type { FileData, FileFormat, FormatHandler } from "../FormatHandler.ts";
import CommonFormats from "src/CommonFormats.ts";
import * as ini from 'ini';
import * as yaml from 'yaml';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused yaml import?

import * as yaml from 'yaml';

class configHandler implements FormatHandler {
public name: string = "cfg";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handler name should be the same as the file and class name. In this case, config, not cfg.

const encoder = new TextEncoder();

const cfgRegex = new RegExp(`\\.(${this.cfgExt.join('|')})$`, 'i');
const yamlRegex = new RegExp(`\\.(${this.yamlExt.join('|')})$`, 'i');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused yaml handling code

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.

2 participants