Whitespace stripped from markdown despite settings, doesn't work with `text.md` scope #185

thoschworks · 2018-12-02T19:31:43Z

Edit by @rsese to describe the larger issue

It's not as much that language-markdown doesn't respect the setting, but whitespace should now also work within the text.md scope.

I believe #177 was supposed to add this functionality so that the whitespace package worked with the text.md scope but as mentioned in #185 (comment) (the author of #177), it looks like it may not have done what was intended.

The result is that if you use language-markdown (as in this report), the whitespace package will still strip trailing whitespace on save in Markdown files even if you have Keep Markdown Line Break Whitespace enabled.

Prerequisites

Put an X between the brackets on this line if you have done all of the following:
- Reproduced the problem in Safe Mode: http://flight-manual.atom.io/hacking-atom/sections/debugging/#using-safe-mode
- Followed all applicable steps in the debugging guide: http://flight-manual.atom.io/hacking-atom/sections/debugging/
- Checked the FAQs on the message board for common solutions: https://discuss.atom.io/c/faq
- Checked that your issue isn't already filed: https://github.com/issues?utf8=✓&q=is%3Aissue+user%3Aatom
- Checked that there is not already an Atom package that provides the described functionality: https://atom.io/packages

Description

Whitespace is stripping trailing whitespaces in markdown files despited the settings.

Edit by @rsese: not reproducible in safe mode because it reproduces when you install and use https://atom.io/packages/language-markdown

Steps to Reproduce

Edit a README.md for a project
Put two whitespaces at end of line to indicate a line break.
Save the file.

Expected behavior: The whitespace at the end of the line are ignored and and not deleted.

Actual behavior: The whitespaces are deleted. (The trailing whitespaces in the current line are ignored.) If the option "Remove Trailing Whitespace" or the package Whitespace is deactivated, the whitespaces remain.

Reproduces how often: 100.0000000%

Versions

Atom : 1.33.0
Electron: 2.0.11
Chrome : 61.0.3163.100
Node : 8.9.3

Whitespace: 0.37.7

OS: Mac OS Mohave 10.14.1

Additional Information

I can not reproduce the issue running Atom on Debian (Bunsen Labs).

The text was updated successfully, but these errors were encountered:

rsese · 2018-12-04T01:14:16Z

Thanks for the report - I'm unable to reproduce with 1.33.0 on macOS 10.12.6. Can you share your settings for Ignore Whitespace On Current Line and Keep Markdown Line Break Whitespace as well?

thoschworks · 2018-12-04T14:15:52Z

Can you share your settings for Ignore Whitespace On Current Line and Keep Markdown Line Break Whitespace as well?

Both are activated.

rsese · 2018-12-05T06:25:29Z

Hmm, I wonder if there's some setting/config somewhere causing the issue? If you temporarily reset to factory defaults, can you still reproduce?

thoschworks · 2018-12-05T08:06:00Z

I have no access to the machine for the next days. Stay tuned…

thoschworks · 2018-12-08T18:25:40Z

After resetting to the factory settings, the problem was solved until I reinstalled the package language-markdown. (On the linux machine is not installed…)

I will report a bug for language-markdown.

thoschworks · 2018-12-08T18:47:13Z

After resetting to the factory settings, the problem was solved until I reinstalled the package language-markdown. (On the linux machine is not installed…)

I will report a bug for language-markdown.

Or perhaps it's the combination of language-markdown and whitespace: language-markdown changes an attribute and whitespace can not recognize the source as markdown?

rsese · 2018-12-11T21:27:52Z

the problem was solved until I reinstalled the package language-markdown. (

There's an existing issue for this on that package repository:

burodepeper/language-markdown#200

Looks like it was closed as resolved in #177 and that change is available in 1.33.0.

@burodepeper 👋 - am I misunderstanding #177? It seems like language-markdown should now respect whitespace's Keep Markdown Line Break Whitespace setting after your change?

burodepeper · 2018-12-12T10:10:11Z

@rsese It's not as much that language-markdown doesn't respect the setting, but whitespace should now also work within the text.md scope. I can't remember to what extend I've tested it. Maybe this can be added to this package's spec?

thoschworks · 2018-12-15T09:19:51Z

@rsese It's not as much that language-markdown doesn't respect the setting, but whitespace should now also work within the text.md scope. I can't remember to what extend I've tested it. Maybe this can be added to this package's spec?

This supports my assumtion, that there is a problem with combination of whitespace and language-markdown.

After adding

".text.md":
    whitespace:
    removeTrailingWhitespace: false

to my config.cson, the problem is solved.

I agree with @burodepeper that whitespace should also respect the scope text.md as markdown file.

I made a mistake in atom#177. For whatever reason, I wrote a quick patch in my browser, and obviously, this code would never yield anything useful, _because it is just silly javascript_. The discussion in atom#185 made me verify my patch. This new patch should actually do something, while I have to admit that I've only verified it via Chrome's console. My test below seems valid enough. ```js const markdownGrammars = ['source.gfm', 'text.md'] let grammarScopeName = 'text.md' markdownGrammars.includes(grammarScopeName) // true grammarScopeName = 'source.js' markdownGrammars.includes(grammarScopeName) // false ```

burodepeper · 2018-12-19T20:10:01Z

I found an issue in my PR in #177 and I've submitted a new PR #186 to fix it. I wrote silly Javascript, I take the blame.

rsese · 2018-12-19T21:15:11Z

Just circling back to this, thanks @burodepeper! I'll edit this issue body today so it's clear the problem is that the whitespace package isn't working with the text.md scope as it was meant to after #177.

ivanpu · 2019-03-17T12:55:05Z

Um... I made a PR for this, but it was attached to the wrong repository... is it possible to migrate it, or should I create a new one from scratch?

burodepeper · 2019-03-17T14:45:12Z

I haven't had time to add tests to my previous PR. If you find time to do it for yours, feel free to do so, then I'll remove my PR.

ivanpu · 2019-03-19T11:16:55Z

I don't know how to add tests, and couldn't find how to test manually on my machine (couldn't find this package files within atom directories). Is there some documentation for this?

rsese · 2019-03-19T20:00:33Z

Our documentation on writing tests is here (it covers running tests as well):

https://flight-manual.atom.io/hacking-atom/sections/writing-specs/

If you need help writing automated tests for Atom, check out Discuss, the official Atom and Electron message board or join the Atom Slack team. There are a bunch of helpful community members that should be willing to point you in the right direction (I'm not an engineer so can't help you myself unfortunately).

ivanpu · 2019-04-18T13:27:48Z

@rsese Thanks, will look at it, hopefully soon - I'm too busy these days with work.

rsese added the more-information-needed label Dec 4, 2018

no-response bot removed the more-information-needed label Dec 4, 2018

rsese added the more-information-needed label Dec 5, 2018

no-response bot removed the more-information-needed label Dec 5, 2018

rsese added the more-information-needed label Dec 5, 2018

no-response bot removed the more-information-needed label Dec 8, 2018

thoschworks closed this as completed Dec 8, 2018

thoschworks reopened this Dec 8, 2018

burodepeper mentioned this issue Dec 19, 2018

Accept multiple markdown scope names #186

Open

rsese added the triaged label Dec 19, 2018

rsese changed the title ~~Whitespace stripped from markdown despite settings, again~~ Whitespace stripped from markdown despite settings, doesn't work with text.md scope Dec 19, 2018

rsese added d ⬇️ i ⬇️ v ⬇️ labels Jan 7, 2019

ivanpu mentioned this issue Mar 17, 2019

Correctly reimplement #177 burodepeper/whitespace#1

Closed

This comment has been minimized.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Whitespace stripped from markdown despite settings, doesn't work with `text.md` scope #185

Whitespace stripped from markdown despite settings, doesn't work with `text.md` scope #185

thoschworks commented Dec 2, 2018 •

edited by rsese

Loading

rsese commented Dec 4, 2018

thoschworks commented Dec 4, 2018

rsese commented Dec 5, 2018

thoschworks commented Dec 5, 2018

thoschworks commented Dec 8, 2018

thoschworks commented Dec 8, 2018

rsese commented Dec 11, 2018

burodepeper commented Dec 12, 2018

thoschworks commented Dec 15, 2018

burodepeper commented Dec 19, 2018

rsese commented Dec 19, 2018

ivanpu commented Mar 17, 2019

burodepeper commented Mar 17, 2019

ivanpu commented Mar 19, 2019

rsese commented Mar 19, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ivanpu commented Apr 18, 2019

Whitespace stripped from markdown despite settings, doesn't work with text.md scope #185

Whitespace stripped from markdown despite settings, doesn't work with text.md scope #185

Comments

thoschworks commented Dec 2, 2018 • edited by rsese Loading

Prerequisites

Description

Steps to Reproduce

Versions

Additional Information

rsese commented Dec 4, 2018

thoschworks commented Dec 4, 2018

rsese commented Dec 5, 2018

thoschworks commented Dec 5, 2018

thoschworks commented Dec 8, 2018

thoschworks commented Dec 8, 2018

rsese commented Dec 11, 2018

burodepeper commented Dec 12, 2018

thoschworks commented Dec 15, 2018

burodepeper commented Dec 19, 2018

rsese commented Dec 19, 2018

ivanpu commented Mar 17, 2019

burodepeper commented Mar 17, 2019

ivanpu commented Mar 19, 2019

rsese commented Mar 19, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ivanpu commented Apr 18, 2019

Whitespace stripped from markdown despite settings, doesn't work with `text.md` scope #185

Whitespace stripped from markdown despite settings, doesn't work with `text.md` scope #185

thoschworks commented Dec 2, 2018 •

edited by rsese

Loading