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

Refactor #100

Merged
merged 22 commits into from
Jan 25, 2019
Merged

Refactor #100

merged 22 commits into from
Jan 25, 2019

Conversation

Cimbali
Copy link
Collaborator

@Cimbali Cimbali commented Jan 18, 2019

Refactor of plugin to make it more readable, fixing bugs (#91 #92 #98) and adding long-standing feature requests (#38 #69 #97).

Also addresses issues raised in ntpeters#98.

Reorder code, organise in sections.
- Simplify utility functions: only keep InitVariable, remove its :exe
- Whitespace patterns and highlighting mechanisms defined once
- Prefs current_line_whitespace_disabled_* can be set only when loading
  the plugin (*), as this allows a lot of simplification.

Clarify variables use/responsability.
- Set buffer-local settings b:better_whitespace_enabled and
  b:strip_whitespace_on_save *only* when Enable/Disable/Toggle is
  called, or when the determination based on the filetype can be made.

- Change SetupAutoCommands to be the only function to be called by:
   a) checking the buffer-local setting
   b) taking responsability for clearing the highlighting
Bug was caused by snyIDattr() returning a string, empty when the
highlight group is undefined, rather than an int with -1 for undefined.
Based on PR ntpeters#69 by @advocateddrummer
Also make sure that EOF stripping of empty lines is only done when the
EOF is in the given range.
Using the number of lines in the file, based on PR ntpeters#97 by @kurikomoe
Also fix docs.
Files that were created in vim printed errors on first save. E.g.:

    :tabe new_file
    :w
Copy link
Owner

@ntpeters ntpeters left a comment

Choose a reason for hiding this comment

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

First off:

Overall, this looks great! Just have a few comments.

One more general note that probably isn't part of this PR since it's pretty big already, but the main plugin script could be cleaned up a bit more by moving all of the functions into an autoload script.

Thanks!

doc/better-whitespace.txt Outdated Show resolved Hide resolved
doc/better-whitespace.txt Show resolved Hide resolved
plugin/better-whitespace.vim Outdated Show resolved Hide resolved
plugin/better-whitespace.vim Outdated Show resolved Hide resolved
plugin/better-whitespace.vim Outdated Show resolved Hide resolved
plugin/better-whitespace.vim Outdated Show resolved Hide resolved
plugin/better-whitespace.vim Outdated Show resolved Hide resolved
plugin/better-whitespace.vim Show resolved Hide resolved
ntpeters and others added 6 commits January 24, 2019 14:41
Use &modified and &modifiable to skip diff and disable strippping
(by default) whitespace on save.
Appending "\n" to joined buffer lines allows to diff until EOF.

Co-Authored-By: Nate Peterson <[email protected]>
doc/better-whitespace.txt Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
Bangs are only used of `:w` for now
That error is the only legitimate one, but is already ignored with the
e flag on :s, see :help s_e
Make commands aware of &readonly and show proper errors.

Also improve StripWhitespaceOnChangedLines to take into account the
range it is passed. The range to defaults to % which means no different
behaviour. With any other range, only perform the stripping on the
intersection of the changed lines and that range.
The explanation says (emphasis mine):

    You tried to execute a command that is neither an Ex command nor
    *a user-defined command*.
@ntpeters
Copy link
Owner

:shipit:

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

Successfully merging this pull request may close these issues.

2 participants