lewang / ws-butler

Unobtrusively trim extraneous white-space *ONLY* in lines edited.
242 stars 26 forks source link

Give user option to skip stripping of whitespace on specific regions #16

Closed xificurC closed 7 years ago

xificurC commented 8 years ago

When writing Elixir code I hit into a little unwanted functionality. Elixir has multiline, heredoc-style strings, e.g.

"""
This is 
a multiline 
string
"""

Trailing whitespace inside this is part of the string, so when stripping it one is effectively changing the string the user put in.

I took the liberty to create this PR in order to provide a quick solution to this. The code checks if the end of line is font-locked as doc or string and if so skips it. This fixed my current issue. If you feel like this is a good-enough solution consider accepting the PR. If not feel free to let me know of any changes I should make to get this accepted or write a different solution if this one is stupid (I'm not an emacs expert).

Thank you for your time in advance.

nixmaniack commented 8 years ago

How about we make this optional instead the default behaviour? I do use multi line strings as well in other languages but deleting trailing whitespace is what's desired.

lewang commented 8 years ago

Instead of mingling custom logic into core ws-butler, I would suggest a user customizable callback that gets beg and end and returns a boolean. Then amend the documentation to show how to use this variable to stop cleanup in this case.

xificurC commented 8 years ago

@nixmaniack @lewang thank you for your feedback. I will try to find some time tomorrow to amend the PR.

xificurC commented 8 years ago

@lewang is this what you had in mind? Please let me know and if so I will add some documentation to it.

lewang commented 8 years ago

Yep, the idea is what I had in mind, barring some implementation issues.

lewang commented 8 years ago

It would also be good to add a test lest we break this feature in the future.

xificurC commented 8 years ago

Thank you for taking your time on this. I am quite busy for the next few days so not sure when will I get back to this. Will take a look at your notes then and try to update the PR.

xificurC commented 8 years ago

Another variation, please let me know if this is what you had in mind. Since you asked the predicate to be affirmative the logic of it is flipped as well. If you accept the changes I'll look into documenting them and see if I can add a test or two.

lewang commented 8 years ago

yes, this looks good to me. Can you add example usage to the README?

xificurC commented 8 years ago

Ah, I didn't understand what you meant by unquoting before, I even checked the docs for funcall and saw everything to be correct. Now I realize it is a variable holding a function and therefore lives in a different namespace. I also had to change fboundp to boundp (didn't push the changes yet). Thanks!

Question: what would be a good example of setting the customizable variable? I don't think setqing is the correct way. Should I just put the lambda that could be set through the customization interface? Or there's customize-save-variable which could be applied through M-:.

lewang commented 8 years ago

I think the example can use setq.

xificurC commented 8 years ago

When using this code the predicate seems to be working fine but inside the tests it's not. If you see what am I doing wrong please let me know, I'm stuck at this for the past 2 days.