-
-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ignore maximum line length #38
base: master
Are you sure you want to change the base?
Conversation
Options: | ||
- IgnoreMaxLineLength: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Options: | |
- IgnoreMaxLineLength: false | |
Validation: | |
IgnoreMaxLineLength: false |
Because the whole file are the options and the "-" marks an array in yaml but this isnt needed here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export class Options {
Validation?: ValidationOptions = new ValidationOptions();
}
export class ValidationOptions {
IgnoreMaxLineLength: boolean = false;
}
this would be the needed models
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it would be better to do something like this:
let options: ValidationOptions = new ValidationOptions();
export function SetValidationOptions(validationOptions: ValidationOptions) {
options = validationOptions;
}
an than use the options here instead of doing this via a parameter
if (conversionYamlOptions) { | ||
const yamlConfig = JSON.parse(JSON.stringify(conversionYamlOptions)).Options; | ||
if (yamlConfig) { | ||
const configOptions: any = Object.assign({}, ...yamlConfig); | ||
ignoreMaxLineLength = configOptions.IgnoreMaxLineLength.toString() === 'true'; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (conversionYamlOptions) { | |
const yamlConfig = JSON.parse(JSON.stringify(conversionYamlOptions)).Options; | |
if (yamlConfig) { | |
const configOptions: any = Object.assign({}, ...yamlConfig); | |
ignoreMaxLineLength = configOptions.IgnoreMaxLineLength.toString() === 'true'; | |
} | |
} | |
const conversionYamlOptions = (yaml.safeLoad(conversionOptions ?? '') as Options) ?? new Options(); | |
SetValidationOptions(conversionYamlOptions.Validation ?? new ValidationOptions()); |
Why so complicated?!
if (conversionYamlOptions) { | ||
const yamlConfig = JSON.parse(JSON.stringify(conversionYamlOptions)).Options; | ||
if (yamlConfig) { | ||
const configOptions: any = Object.assign({}, ...yamlConfig); | ||
ignoreMaxLineLength = configOptions.IgnoreMaxLineLength.toString() === 'true'; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
@@ -151,7 +186,7 @@ function ExecuteParseFile( | |||
|
|||
lr.on('line', function (line: any) { | |||
lr.pause(); | |||
ProcessNewLine(lastLevel, lineNumber, line, nextLine); | |||
ProcessNewLine(lastLevel, lineNumber, line, nextLine, ignoreMaxLineLength); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alle parameters like this are not needed anymore
Add an option to allow GEDCOM lines to be longer than 250 characters.
Normally, lines longer than this would fail a validation check, but depending on requirements, it might be OK to allow longer lines in incoming GEDCOMs.
The option is supplied by a new config setting
conversion-options
which points to a yaml file.If the yaml file is not supplied, the included file is used and the setting defaults to false.
Example:
=> options/conversion.yaml
=> examples/longAddress.ged
RESULT: