-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Fix #1879 Add .md extension to files created in sidebar if no extension is given #1886
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
Conversation
src/renderer/store/project.js
Outdated
@@ -173,7 +173,14 @@ const actions = { | |||
|
|||
CREATE_FILE_DIRECTORY ({ commit, state }, name) { | |||
const { dirname, type } = state.createCache | |||
|
|||
if (type === 'file') { | |||
const CHECK_MD_EXTENSION = /.*\.(?:markdown|mdown|mkdn|md|mkd|mdwn|mdtxt|mdtext|text|txt)$/i |
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.
What about people that use RMarkdown (.rmd
) or use .pandoc
as their file extension? Hard coding these seems like an ugly hack that will never cover all the bases. How about just checking if there is any extension at all?
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 agree to you, but the problem is, that the sidebar does not allow other extensions in the first place. Have a look at src/common/filesystem/paths.js
export const MARKDOWN_EXTENSIONS = [
'markdown',
'mdown',
'mkdn',
'md',
'mkd',
'mdwn',
'mdtxt',
'mdtext',
'text',
'txt'
]
// ...
/**
* Returns true if the filename matches one of the markdown extensions.
*
* @param {string} filename Path or filename
*/
export const hasMarkdownExtension = filename => {
if (!filename || typeof filename !== 'string') return false
return MARKDOWN_EXTENSIONS.some(ext => filename.toLowerCase().endsWith(`.${ext}`))
}
If you create for example a .pandoc
file in the sidebar, it is created but Mark Text does not allow to open it and greys it out. That's the reason why I decided to check for these extensions as well.
Maybe we could add the default .md
extension if no extension is given at all, and warn the user if he creates a file with a disallowed extension?
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.
Okay, I guess it would be acceptable for now to match what the sidebar already does, but both the sidebar and this should probably be expanded to cover more options in the future. Would it be possible to use a single constant for this so it doesn't need to be updated in two places and create possible miss-mataches?
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.
Thanks for the Feedback! There are several places where either the regex I used is used or markdown extensions are matched another way. Maybe we should consider to use the |
…hecked. Allowed markdown extensions are handeled in several places in different ways. Use the hasMarkdownFunction from common/filesystem/paths.js to allow to easily update the allowed extensions when needed.
Good idea, will you change them in this PR? |
Description
Fixes Issue #1879
--
Please don't submit
/dist
files with your PR!