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

fix markdown code for checked checkbox #1070

Closed
wants to merge 1 commit into from

Conversation

tessus
Copy link
Collaborator

@tessus tessus commented Dec 24, 2018

fixes #1067

@tessus tessus added bug It's a bug essential labels Dec 24, 2018
@tessus
Copy link
Collaborator Author

tessus commented Dec 24, 2018

@laurent22 I used the same labels as for the corresponding ticket. Please let me know, if this is a good practice or if you prefer not to have any labels for PRs.

body = body.replace(/°°JOP°CHECKBOX°NOTICK°°/g, '- [ ]');
body = body.replace(/°°JOP°CHECKBOX°TICK°°/g, '- [X]');
body = body.replace(/°°JOP°CHECKBOX°NOTICK°°/g, '- [ ]');
body = body.replace(/°°JOP°CHECKBOX°TICK°°/g, '- [x]');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the real fix.

@tessus
Copy link
Collaborator Author

tessus commented Dec 24, 2018

Sorry, my editor removed trailing spaces again.

@laurent22
Copy link
Owner

Looks good, but the whitespace changes need to fix per the contributing guidelines.

@tessus
Copy link
Collaborator Author

tessus commented Jan 9, 2019

Well, not sure how to tell my editor not to fix whitespace errors. I wil have to look into my editor settings, but I think these settings have to be changed in a config file not in the GUI (Sublime Text). But this will take some time. In vim there's no autocmd in my .vimrc, yet trailing spaces are still removed. I have no glue where this is coming from.

Closing the PR for now. But I'm rather inclined to stop contributing all together. At least when it comes to PRs.
(Sorry, this is nothing personal, but I'm not wasting my time in supporting a project's idiosyncrasy when it comes to whitespace errors. Remember, I suggested that you fixed all whitespace errors yourself in one single commit, and/or offered to do this myself and create a PR.)

@tessus tessus closed this Jan 9, 2019
@Outi-s
Copy link

Outi-s commented Jan 9, 2019

Sublime text, in Preferences > Settings: "trim_automatic_white_space": false, & "trim_trailing_white_space_on_save": false,

Does this help?

@laurent22
Copy link
Owner

Sorry to hear that, it's unfortunate if the guidelines prevent you from contributing but at this stage it's quite low priority to change them. As subi54 mentioned there are settings that can be tweaked to make sure the editor doesn't modify unrelated code. In ST case I think it can even be done per project, so for example white space trimming would be disabled for Joplin but not for other code you work on.

@tessus
Copy link
Collaborator Author

tessus commented Jan 9, 2019

@subi54 Yes, this works. Thank you very much.

I will update this PR later tonight and re-open it.

@laurent22 I understand that this topic is a low priority for you and that's why I offered to fix it and create a PR. This PR won't change the code and thus won't break anything. Everything will be the same as before, except that nobody will ever run into such an issue again.

The only thing you have to do is to press the merge button. Takes about a second to do so.

Especially for js projects using a linter in the CI pipeline is a very easy change, but it would reject PRs that include whitespace errors. This would be the next logical step. Even if you don't want to do this now, I could fix whitespace errors manually and commit them, so there's no extra work for you. And since I don't change code, you can be sure that these whitespace changes won't break anything.
A win-win situation so to speak.

Even, if you don't want to handle this now, please keep my offer in mind and ping me as soon as you are ready to get this done.

@laurent22
Copy link
Owner

@tessus, I would be fine with a linter that does only three things: 1. use tabs, 2. trim white spaces, 3. add a new line at end of file. I've tried at some point to get something like this working but it had default settings I guess that also wrapped lines and made various other changes that I didn't want.

So if you can figure out the settings for a simple linter that work let's do that (but after the current roadmap is completed), and we'll also need to figure out how to integrate it - either via CI or post-commit hook, whatever is simpler.

@Outi-s
Copy link

Outi-s commented Jan 9, 2019

@laurent, yep per project, per syntax, however you want. Sublime Text is versatile that way.
Why not use regex to do those things? I don't understand lint but those issues seems like something regex could fix.

@tessus:

Be default Ctrl+R or the Goto Symbolcan be use to navigate Markdown headings.

save_on_focus_lost, spelling one or as mentioned above trim spaces settings are a must.

Snippets are neat as are packages. You can find one for almost anything.

Play around, you'll love it how much time and energy you could save with lil things here and there. I don't even know code, I use all these just for notes. :)

@tessus
Copy link
Collaborator Author

tessus commented Jan 10, 2019

Hmm, weird, I can't reopen the PR. -> #1113

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It's a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

markdown code for checked checkbox
3 participants