Skip to content
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

Update Escape Logic and Fix Issue with Failure to Escape Titles and Aliases #461

Merged
merged 3 commits into from
Oct 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 59 additions & 7 deletions __tests__/yaml-title-alias.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -432,17 +432,48 @@ ruleTest({
},
},
{
testName: 'Titles with special characters are escaped',
testName: 'Titles with special a colon and then a space are escaped',
before: dedent`
# Title with: colon, 'quote', "single quote"
# Title with: colon
`,
after: dedent`
---
aliases:
- 'Title with: colon, ''quote'', "single quote"'
linter-yaml-title-alias: 'Title with: colon, ''quote'', "single quote"'
- 'Title with: colon'
linter-yaml-title-alias: 'Title with: colon'
---
# Title with: colon, 'quote', "single quote"
# Title with: colon
`,
options: {
defaultEscapeCharacter: '\'',
},
},
{
testName: 'Titles with double quote are escaped',
before: dedent`
# Title with "double quote"
`,
after: dedent`
---
aliases:
- 'Title with "double quote"'
linter-yaml-title-alias: 'Title with "double quote"'
---
# Title with "double quote"
`,
},
{
testName: 'Titles with single quote are escaped',
before: dedent`
# Title with 'single quote'
`,
after: dedent`
---
aliases:
- "Title with 'single quote'"
linter-yaml-title-alias: "Title with 'single quote'"
---
# Title with 'single quote'
`,
},
{
Expand Down Expand Up @@ -1028,8 +1059,8 @@ ruleTest({
after: dedent`
---
aliases:
- '[[Heading]]'
linter-yaml-title-alias: '[[Heading]]'
- [[Heading]]
linter-yaml-title-alias: [[Heading]]
---
[[Link1]]

Expand All @@ -1039,5 +1070,26 @@ ruleTest({
aliasArrayStyle: NormalArrayFormats.MultiLine,
},
},
{ // accounts for https://github.com/platers/obsidian-linter/issues/439
testName: 'Make sure escaped aliases that match the H1 do not get added back',
before: dedent`
---
aliases:
- "It's strange"
---
# It's strange
`,
after: dedent`
---
aliases:
- "It's strange"
---
# It's strange
`,
options: {
aliasArrayStyle: NormalArrayFormats.MultiLine,
useYamlKeyToKeepTrackOfOldFilenameOrHeading: false,
},
},
],
});
6 changes: 3 additions & 3 deletions __tests__/yaml-title.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ ruleTest({
`,
after: dedent`
---
title: 'Hello: world'
title: "Hello: world"
---
# Hello: world
`,
Expand All @@ -36,7 +36,7 @@ ruleTest({
`,
after: dedent`
---
title: '''Hello world'
title: "'Hello world"
---
# 'Hello world
`,
Expand Down Expand Up @@ -74,7 +74,7 @@ ruleTest({
`,
after: dedent`
---
title: '[[Heading]]'
title: [[Heading]]
---
[[Link1]]

Expand Down
36 changes: 3 additions & 33 deletions src/rules/escape-yaml-special-characters.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {Options, RuleType} from '../rules';
import RuleBuilder, {BooleanOptionBuilder, ExampleBuilder, OptionBuilderBase} from './rule-builder';
import dedent from 'ts-dedent';
import {formatYAML, isValueEscapedAlready} from '../utils/yaml';
import {escapeStringIfNecessaryAndPossible, formatYAML} from '../utils/yaml';

class EscapeYamlSpecialCharactersOptions implements Options {
@RuleBuilder.noSettingControl()
Expand Down Expand Up @@ -32,36 +32,6 @@ export default class EscapeYamlSpecialCharacters extends RuleBuilder<EscapeYamlS
return text;
}

const escapeSubstringIfNecessary = function(fullText: string, substring: string): string {
if (isValueEscapedAlready(substring)) {
return fullText;
}

// if there is no single quote, double quote, or colon to escape, skip this substring
const substringHasSingleQuote = substring.includes('\'');
const substringHasDoubleQuote = substring.includes('"');
const substringHasColonWithSpaceAfterIt = substring.includes(': ');
if (!substringHasSingleQuote && !substringHasDoubleQuote && !substringHasColonWithSpaceAfterIt) {
return fullText;
}

// if the substring already has a single quote and a double quote, there is nothing that can be done to escape the substring
if (substringHasSingleQuote && substringHasDoubleQuote) {
return fullText;
}

let newText: string;
if (substringHasSingleQuote) {
newText = fullText.replace(substring, `"${substring}"`);
} else if (substringHasDoubleQuote) {
newText = fullText.replace(substring, `'${substring}'`);
} else { // the line must have a colon with a space
newText = fullText.replace(substring, `${options.defaultEscapeCharacter}${substring}${options.defaultEscapeCharacter}`);
}

return newText;
};

for (let i = 0; i < yamlLineCount; i++) {
const line = yamlLines[i].trim();

Expand Down Expand Up @@ -99,7 +69,7 @@ export default class EscapeYamlSpecialCharacters extends RuleBuilder<EscapeYamlS
arrayItem = arrayItem.substring(0, arrayItem.length - 1).trimEnd();
}

arrayItems[j] = escapeSubstringIfNecessary(arrayItems[j], arrayItem);
arrayItems[j] = arrayItems[j].replace(arrayItem, escapeStringIfNecessaryAndPossible(arrayItem, options.defaultEscapeCharacter));
}

yamlLines[i] = yamlLines[i].replace(value, '[' + arrayItems.join(',') + ']');
Expand All @@ -108,7 +78,7 @@ export default class EscapeYamlSpecialCharacters extends RuleBuilder<EscapeYamlS
continue;
}

yamlLines[i] = escapeSubstringIfNecessary(yamlLines[i], value);
yamlLines[i] = yamlLines[i].replace(value, escapeStringIfNecessaryAndPossible(value, options.defaultEscapeCharacter));
}

return yamlLines.join('\n');
Expand Down
7 changes: 4 additions & 3 deletions src/rules/force-yaml-escape.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {Options, RuleType} from '../rules';
import RuleBuilder, {ExampleBuilder, OptionBuilderBase, TextAreaOptionBuilder} from './rule-builder';
import dedent from 'ts-dedent';
import {formatYAML, getYamlSectionValue, isValueEscapedAlready, setYamlSection} from '../utils/yaml';
import {escapeStringIfNecessaryAndPossible, formatYAML, getYamlSectionValue, isValueEscapedAlready, setYamlSection} from '../utils/yaml';

class ForceYamlEscapeOptions implements Options {
@RuleBuilder.noSettingControl()
Expand All @@ -26,15 +26,16 @@ export default class ForceYamlEscape extends RuleBuilder<ForceYamlEscapeOptions>
apply(text: string, options: ForceYamlEscapeOptions): string {
return formatYAML(text, (text) => {
for (const yamlKeyToEscape of options.forceYamlEscape) {
const keyValue = getYamlSectionValue(text, yamlKeyToEscape);
let keyValue = getYamlSectionValue(text, yamlKeyToEscape);

if (keyValue != null) {
// skip yaml array values or already escaped values
if (keyValue.includes('\n') || keyValue.startsWith(' [') || isValueEscapedAlready(keyValue)) {
continue;
}

text = setYamlSection(text, yamlKeyToEscape, ` ${options.defaultEscapeCharacter}${keyValue}${options.defaultEscapeCharacter}`);
keyValue = escapeStringIfNecessaryAndPossible(keyValue, options.defaultEscapeCharacter, true);
text = setYamlSection(text, yamlKeyToEscape, ' ' + keyValue);
}
}

Expand Down
12 changes: 8 additions & 4 deletions src/rules/yaml-title-alias.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
import {Options, RuleType} from '../rules';
import RuleBuilder, {BooleanOptionBuilder, ExampleBuilder, OptionBuilderBase} from './rule-builder';
import dedent from 'ts-dedent';
import {convertAliasValueToStringOrStringArray, formatYamlArrayValue, getYamlSectionValue, initYAML, LINTER_ALIASES_HELPER_KEY, loadYAML, NormalArrayFormats, OBSIDIAN_ALIASES_KEY, removeYamlSection, setYamlSection, SpecialArrayFormats, splitValueIfSingleOrMultilineArray, toYamlString} from '../utils/yaml';
import {convertAliasValueToStringOrStringArray, escapeStringIfNecessaryAndPossible, formatYamlArrayValue, getYamlSectionValue, initYAML, LINTER_ALIASES_HELPER_KEY, loadYAML, NormalArrayFormats, OBSIDIAN_ALIASES_KEY, removeYamlSection, setYamlSection, SpecialArrayFormats, splitValueIfSingleOrMultilineArray} from '../utils/yaml';
import {ignoreListOfTypes, IgnoreTypes} from '../utils/ignore-types';
import {yamlRegex} from '../utils/regex';


class YamlTitleAliasOptions implements Options {
@RuleBuilder.noSettingControl()
aliasArrayStyle?: NormalArrayFormats | SpecialArrayFormats = NormalArrayFormats.MultiLine;
preserveExistingAliasesSectionStyle?: boolean = true;
keepAliasThatMatchesTheFilename?: boolean = false;
useYamlKeyToKeepTrackOfOldFilenameOrHeading?: boolean = true;

@RuleBuilder.noSettingControl()
aliasArrayStyle?: NormalArrayFormats | SpecialArrayFormats = NormalArrayFormats.MultiLine;

@RuleBuilder.noSettingControl()
fileName?: string;

@RuleBuilder.noSettingControl()
defaultEscapeCharacter?: string = '"';
}

@RuleBuilder.register
Expand Down Expand Up @@ -55,6 +59,7 @@ export default class YamlTitleAlias extends RuleBuilder<YamlTitleAliasOptions> {

previousTitle = loadYAML(getYamlSectionValue(yaml, LINTER_ALIASES_HELPER_KEY));

title = escapeStringIfNecessaryAndPossible(title, options.defaultEscapeCharacter);
const getNewAliasValue = function(originalValue: string |string[], shouldRemoveTitle: boolean): string |string[] {
if (originalValue == null) {
return shouldRemoveTitle ? '' : title;
Expand Down Expand Up @@ -97,7 +102,6 @@ export default class YamlTitleAlias extends RuleBuilder<YamlTitleAliasOptions> {
return originalValue;
};

title = toYamlString(title);
if (Object.keys(parsedYaml).includes(OBSIDIAN_ALIASES_KEY)) {
const aliasesValue = getYamlSectionValue(newYaml, OBSIDIAN_ALIASES_KEY);
let currentAliasStyle: NormalArrayFormats | SpecialArrayFormats = NormalArrayFormats.MultiLine;
Expand Down
7 changes: 5 additions & 2 deletions src/rules/yaml-title.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {Options, RuleType} from '../rules';
import RuleBuilder, {ExampleBuilder, OptionBuilderBase, TextOptionBuilder} from './rule-builder';
import dedent from 'ts-dedent';
import {formatYAML, initYAML, toYamlString} from '../utils/yaml';
import {escapeStringIfNecessaryAndPossible, formatYAML, initYAML} from '../utils/yaml';
import {ignoreListOfTypes, IgnoreTypes} from '../utils/ignore-types';
import {escapeDollarSigns} from '../utils/regex';
import {insert} from '../utils/strings';
Expand All @@ -10,6 +10,9 @@ class YamlTitleOptions implements Options {
@RuleBuilder.noSettingControl()
fileName: string;

@RuleBuilder.noSettingControl()
defaultEscapeCharacter?: string = '"';

titleKey?: string = 'title';
}

Expand Down Expand Up @@ -38,7 +41,7 @@ export default class YamlTitle extends RuleBuilder<YamlTitleOptions> {
});
title = title || options.fileName;

title = toYamlString(title);
title = escapeStringIfNecessaryAndPossible(title, options.defaultEscapeCharacter);

return formatYAML(text, (text) => {
const title_match_str = `\n${options.titleKey}.*\n`;
Expand Down
46 changes: 37 additions & 9 deletions src/utils/yaml.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {load, dump} from 'js-yaml';
import {load} from 'js-yaml';
import {escapeDollarSigns, yamlRegex} from './regex';

export const OBSIDIAN_TAG_KEY = 'tags';
Expand Down Expand Up @@ -29,14 +29,6 @@ export function formatYAML(text: string, func: (text: string) => string): string
return text;
}

export function toYamlString(obj: any): string {
return dump(obj, {lineWidth: -1}).slice(0, -1);
}

export function toSingleLineArrayYamlString<T>(arr: T[]): string {
return dump(arr, {flowLevel: 0}).slice(0, -1);
}

function getYamlSectionRegExp(rawKey: string): RegExp {
return new RegExp(`^([\\t ]*)${rawKey}:[ \\t]*(\\S.*|(?:(?:\\n *- \\S.*)|((?:\\n *- *))*|(\\n([ \\t]+[^\\n]*))*)*)\\n`, 'm');
}
Expand Down Expand Up @@ -259,3 +251,39 @@ export function isValueEscapedAlready(value: string): boolean {
return value.length > 1 && ((value.startsWith('\'') && value.endsWith('\'')) ||
(value.startsWith('"') && value.endsWith('"')));
}

/**
* Escapes the provided string value if it has a colon with a space after it, a single quote, or a double quote, but not a single and double quote.
* @param {string} value The value to escape if possible
* @param {string} defaultEscapeCharacter The character escape to use around the value if a specific escape character is not needed.
* @param {boolean} forceEscape Whether or not to force the escaping of the value provided.
* @return {string} The escaped value if it is either necessary or forced and the provided value if it cannot be escaped, is escaped,
* or does not need escaping and the force escape is not used.
*/
export function escapeStringIfNecessaryAndPossible(value: string, defaultEscapeCharacter: string, forceEscape: boolean = false): string {
if (isValueEscapedAlready(value)) {
return value;
}

// if there is no single quote, double quote, or colon to escape, skip this substring
const substringHasSingleQuote = value.includes('\'');
const substringHasDoubleQuote = value.includes('"');
const substringHasColonWithSpaceAfterIt = value.includes(': ');
if (!substringHasSingleQuote && !substringHasDoubleQuote && !substringHasColonWithSpaceAfterIt && !forceEscape) {
return value;
}

// if the substring already has a single quote and a double quote, there is nothing that can be done to escape the substring
if (substringHasSingleQuote && substringHasDoubleQuote) {
return value;
}

if (substringHasSingleQuote) {
return `"${value}"`;
} else if (substringHasDoubleQuote) {
return `'${value}'`;
}

// the line must have a colon with a space
return `${defaultEscapeCharacter}${value}${defaultEscapeCharacter}`;
}