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

Title will now be extracted from front matter if title: is present. #2510

Merged
merged 2 commits into from
Oct 23, 2018
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
31 changes: 19 additions & 12 deletions browser/lib/findNoteTitle.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
export function findNoteTitle (value) {
export function findNoteTitle (value, enableFrontMatterTitle, frontMatterTitleField = 'title') {
const splitted = value.split('\n')
let title = null
let isInsideCodeBlock = false

if (splitted[0] === '---') {
let line = 0
while (++line < splitted.length) {
if (enableFrontMatterTitle && splitted[line].startsWith(frontMatterTitleField + ':')) {
title = splitted[line].substring(frontMatterTitleField.length + 1).trim()

break
}
if (splitted[line] === '---') {
splitted.splice(0, line + 1)

Expand All @@ -14,17 +19,19 @@ export function findNoteTitle (value) {
}
}

splitted.some((line, index) => {
const trimmedLine = line.trim()
const trimmedNextLine = splitted[index + 1] === undefined ? '' : splitted[index + 1].trim()
if (trimmedLine.match('```')) {
isInsideCodeBlock = !isInsideCodeBlock
}
if (isInsideCodeBlock === false && (trimmedLine.match(/^# +/) || trimmedNextLine.match(/^=+$/))) {
title = trimmedLine
return true
}
})
if (title === null) {
splitted.some((line, index) => {
const trimmedLine = line.trim()
const trimmedNextLine = splitted[index + 1] === undefined ? '' : splitted[index + 1].trim()
if (trimmedLine.match('```')) {
isInsideCodeBlock = !isInsideCodeBlock
}
if (isInsideCodeBlock === false && (trimmedLine.match(/^# +/) || trimmedNextLine.match(/^=+$/))) {
title = trimmedLine
return true
}
})
}

if (title === null) {
title = ''
Expand Down
2 changes: 1 addition & 1 deletion browser/main/Detail/MarkdownNoteDetail.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class MarkdownNoteDetail extends React.Component {
handleUpdateContent () {
const { note } = this.state
note.content = this.refs.content.value
note.title = markdown.strip(striptags(findNoteTitle(note.content)))
note.title = markdown.strip(striptags(findNoteTitle(note.content, this.props.config.editor.enableFrontMatterTitle, this.props.config.editor.frontMatterTitleField)))
this.updateNote(note)
}

Expand Down
2 changes: 1 addition & 1 deletion browser/main/Detail/SnippetNoteDetail.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class SnippetNoteDetail extends React.Component {
if (this.refs.tags) note.tags = this.refs.tags.value
note.description = this.refs.description.value
note.updatedAt = new Date()
note.title = findNoteTitle(note.description)
note.title = findNoteTitle(note.description, false)

this.setState({
note
Expand Down
4 changes: 3 additions & 1 deletion browser/main/lib/ConfigManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ export const DEFAULT_CONFIG = {
scrollPastEnd: false,
type: 'SPLIT',
fetchUrlTitle: true,
enableTableEditor: false
enableTableEditor: false,
enableFrontMatterTitle: true,
frontMatterTitleField: 'title'
},
preview: {
fontSize: '14',
Expand Down
29 changes: 28 additions & 1 deletion browser/main/modals/PreferencesModal/UiTab.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ class UiTab extends React.Component {
snippetDefaultLanguage: this.refs.editorSnippetDefaultLanguage.value,
scrollPastEnd: this.refs.scrollPastEnd.checked,
fetchUrlTitle: this.refs.editorFetchUrlTitle.checked,
enableTableEditor: this.refs.enableTableEditor.checked
enableTableEditor: this.refs.enableTableEditor.checked,
enableFrontMatterTitle: this.refs.enableFrontMatterTitle.checked,
frontMatterTitleField: this.refs.frontMatterTitleField.value
Copy link
Member

Choose a reason for hiding this comment

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

I think you should trim() this string first. If I leave a leading blank space the title will be broken because <space>title != title and you should add test for that too 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I think that change would definitely make sense but I'm not sure where to add it. It doesn't look like any of the other fields do any kind of validation/cleanup currently either. For example, I can change "Editor Font Size" to "12a" and I can change "LaTeX Inline Open Delimiter" to " $ " (two leading and two trailing spaces).

I also don't see that there's any existing test infrastructure for the UI tab or for the underlying ConfigManager.

Maybe that kind of stuff should all be done as a separate set of changes?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm...I guess you're right. I approve your change and maybe open an issue for it.

},
preview: {
fontSize: this.refs.previewFontSize.value,
Expand Down Expand Up @@ -428,6 +430,31 @@ class UiTab extends React.Component {
</div>
</div>

<div styleName='group-section'>
<div styleName='group-section-label'>
{i18n.__('Front matter title field')}
</div>
<div styleName='group-section-control'>
<input styleName='group-section-control-input'
ref='frontMatterTitleField'
value={config.editor.frontMatterTitleField}
onChange={(e) => this.handleUIChange(e)}
type='text'
/>
</div>
</div>

<div styleName='group-checkBoxSection'>
<label>
<input onChange={(e) => this.handleUIChange(e)}
checked={this.state.config.editor.enableFrontMatterTitle}
ref='enableFrontMatterTitle'
type='checkbox'
/>&nbsp;
{i18n.__('Extract title from front matter')}
</label>
</div>

<div styleName='group-checkBoxSection'>
<label>
<input onChange={(e) => this.handleUIChange(e)}
Expand Down
42 changes: 41 additions & 1 deletion tests/lib/find-title-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,47 @@ test('findNoteTitle#find should return a correct title (string)', t => {

testCases.forEach(testCase => {
const [input, expected] = testCase
t.is(findNoteTitle(input), expected, `Test for find() input: ${input} expected: ${expected}`)
t.is(findNoteTitle(input, false), expected, `Test for find() input: ${input} expected: ${expected}`)
})
})

test('findNoteTitle#find should ignore front matter when enableFrontMatterTitle=false', t => {
// [input, expected]
const testCases = [
['---\nlayout: test\ntitle: hoge hoge hoge \n---\n# fuga', '# fuga'],
['---\ntitle:hoge\n---\n# fuga', '# fuga'],
['title: fuga\n# hoge', '# hoge']
]

testCases.forEach(testCase => {
const [input, expected] = testCase
t.is(findNoteTitle(input, false), expected, `Test for find() input: ${input} expected: ${expected}`)
})
})

test('findNoteTitle#find should respect front matter when enableFrontMatterTitle=true', t => {
// [input, expected]
const testCases = [
['---\nlayout: test\ntitle: hoge hoge hoge \n---\n# fuga', 'hoge hoge hoge'],
['---\ntitle:hoge\n---\n# fuga', 'hoge'],
['title: fuga\n# hoge', '# hoge']
]

testCases.forEach(testCase => {
const [input, expected] = testCase
t.is(findNoteTitle(input, true), expected, `Test for find() input: ${input} expected: ${expected}`)
})
})

test('findNoteTitle#find should respect frontMatterTitleField when provided', t => {
// [input, expected]
const testCases = [
['---\ntitle: hoge\n---\n# fuga', '# fuga'],
['---\ncustom: hoge\n---\n# fuga', 'hoge']
]

testCases.forEach(testCase => {
const [input, expected] = testCase
t.is(findNoteTitle(input, true, 'custom'), expected, `Test for find() input: ${input} expected: ${expected}`)
})
})