-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Enable Markdown lint option #3034
Conversation
…stnote into enable-markdownlint-option
…ke at allow custom css)
browser/components/CodeEditor.js
Outdated
Jsonlint.parse(customMarkdownLintConfig) | ||
lintConfigJson = JSON.parse(customMarkdownLintConfig) | ||
} catch (err) { | ||
throw err |
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 don't think throwing an error here is a good thing to do because it will make the app unusable. I think you should put something more "user-friendly" for example not applying the lint config and display error message above the custom lint config box in the setting.
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.
Yes, you're right. It's bad to throw an error with-out handling it.
How can I add the error message display? I've tried to change the throw err
to
eventEmitter.emit('APP_SETTING_ERROR')
return
But I don't like how it's working. It first shows success message and then the error occured message. It happens pretty fast - short flicker of the green message. Also on a second click it won't show the error again.
The catch
can be triggered by entering invalid json for the custom markdown lint rule and then press the save button.
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.
A custom error message wasn't working - not sure why. I wanted to add something like: Failed to apply custom linting rule.
Tried it with eventEmitter.emit('APP_SETTING_ERROR', {message: 'Failed to apply...'})
And please remove all |
213116e
to
c70cca2
Compare
Submit this pr to https://issuehunt.io/r/BoostIO/Boostnote/issues/3038 :) |
Thank you! I've enabled linting and configured it the way I want. Thank you for being open to the feedback. |
After merging #2983 we can add this to toggle markdownLinting.
I've based this on branch
roottool:add-markdownlint-rules-form
from PR #2983Description
enableMarkdownLint
settingenableMarkdownLint
Issue fixed
#3025
Type of changes
Checklist:
Todos:
Discussion
enableMarkdownLint
to false by default OK?style.display
and keep it in the config. Not sure if there is a better way to handle it but it's working now.Screenshot
Current setting
Below is the location from my first draft (changed it to the above location)