Force whitespace pre-save #47

Closed
wants to merge 3 commits into
from

Projects

None yet

4 participants

@kalebtrotter

Removing the spaces == True and spaces == False check in the apply_pre_save method simply tells Sublime to enforce the tabs/spaces style throughout the document on save, as requested in #33

@ktrotter-emsi ktrotter-emsi Force whitespace pre-save
Removing the ‘spaces == True/False’ check in the apply_pre_save method
allows Sublime to enforce the tabs/spaces style throughout the
document, as requested in issue #33
3c5e359
@sindresorhus
Owner

It's like it is for a reason. git blame is a good tool to figure out why code is like it is.

0a296cc

@kalebtrotter

Thanks for clarifying! Sorry for not doing my research -- definitely should have looked into why the check was present.

I've noticed those scroll position changes when using translate_tabs_to_spaces, but in my experience (SublimeText 3 on Mac), it didn't move by much. In my mind, the benefits of the change far outweigh what has been a pretty trivial scroll position change (again, that's just my experience and the experience of a few co-workers).

Totally understand that the scroll issue may be a more significant/annoying problem for other users. Happy to have this request closed if the solution isn't acceptable, thanks much for considering.

@sindresorhus
Owner

I'm honestly not sure. I'm leaning towards merging this, but there was a lot of user complaining about the "jump" #27. Too bad Sublime Text isn't open source so we could fix the actual bug...

In my mind, the benefits of the change far outweigh what has been a pretty trivial scroll position change

Do you know if it's possible to manually set the scroll position? If so we could maybe come up with a hack that prevents the jump.

@kalebtrotter

Sorry about the lengthy hiatus on this, went on a long vacation and have been catching up with work since. I like manually setting scroll position as a work-around, and I'll keep exploring that option. Thanks for the reply!

kalebtrotter added some commits Jul 16, 2015
@kalebtrotter kalebtrotter Maintain viewport position when translating tabs
A workaround to maintain current viewport position when running the
‘set_translate_tabs’ command.
057930a
@kalebtrotter kalebtrotter Removing unnecessary param
Turns out the False I was passing to prevent animating the position
resetting wasn’t necessary
8b103c8
@kalebtrotter

@sindresorhus Finally got some time to work on this, and it was super straightforward. Manually setting the viewport worked like a charm.

@brismuth

I've been using this branch for a bit and I haven't had any issues. Great fix @kalebtrotter.

@kalebtrotter

@brismuth Thanks! Glad to hear it!

@sindresorhus
Owner

@kalebtrotter Nice! Thanks :)

@kalebtrotter

Thank you! Glad I could help out even a little bit. Cheers!

@sindresorhus
Owner

🍻 Cheers!

And if you're looking for more challenges I could always use some help with #44 ;)

@sindresorhus sindresorhus added a commit that referenced this pull request Jul 26, 2015
@sindresorhus Revert "Close #47 PR: Force whitespace pre-save. Fixes #33"
This reverts commit 1912349.
07b8896
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment