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

Is appendArtifactFix still needed? #225

Closed
slabua opened this issue Jan 13, 2018 · 6 comments
Closed

Is appendArtifactFix still needed? #225

slabua opened this issue Jan 13, 2018 · 6 comments
Milestone

Comments

@slabua
Copy link

slabua commented Jan 13, 2018

let appendArtifactFix = 1

I had a look at issues #58 and #197 which seemed related to this variable, it took me a while to find it hidden in the code while I was trying to remove a nbsp from showing up next to the file type icons in NERDTree. I only realised it because I use listchars to show also the nbsp's.
I have now set appendArtifactFix to 0 and I didn't encounter any drawbacks.
Could anyone check whether this variable still needs to be set to 1, or perhaps consider to provide a variable to be set in .vimrc?

@ryanoasis
Copy link
Owner

ryanoasis commented Jan 25, 2018

or perhaps consider to provide a variable to be set in .vimrc

Yeah I think this is a very sensible improvement to make.

still needs to be set to 1 ?

So it has been a while since I added that logic and trying to remember why I added it in with other changes to fix 'Folder titles off by a single space after opening' #197

So the "fix" was/is very hacky and now looking it makes me blush. Anyway I believe that was added only for problems with the glyphs being cut off in Gui Vim implementations.

Plan of action

To flag or not to flag

  • Determine necessity of flag
    • if no obvious discrepancies across OS and Gui vs. terminal change default to off (0)
      • perhaps default to on (1) on gui?
    • if there are any discrepancies leave as default of on (1)

Make configurable

  • Make the option configurable
    • We should be able to both turn or off the option as well as specify the unicode hex
      • something akin to:
        " Default of 0 meaning off, 
        " any string value would mean that is used as the character
        " to print after glyphs
        if !exists('g:DevIconsArtifactFix')
          let g:DevIconsArtifactFix = 0
        endif

Ensure consistent use of padding

  • Ensure consistent use of g:WebDevIconsNerdTreeAfterGlyphPadding throughout the code
    • Currently not consistently used.. causing 'bugs'

Test


I would request and require some help testing, particularly in MacOS land..

I would also ideally like to ask @ismay if they mind to help ensure no regression to fix reported. Barring that we can test using the settings reported in the issue (#197)

@ryanoasis
Copy link
Owner

I think we are probably good with this now at least it should be consistent 🤞

I will do more testing tomorrow, if anyone else could test latest master I would greatly appreciate it ❤️

@ryanoasis
Copy link
Owner

ryanoasis commented Mar 11, 2018

List to check off tomorrow:

@kutsan
Copy link
Contributor

kutsan commented Mar 11, 2018

basic NERDTree actions: opening, closing, changing root of folders, closing/opening NERDTree

Get that squared away. I've been using it with my local fix heavily for at least 20 days without any problem.

@ryanoasis
Copy link
Owner

@kutsan Yeah I figure it's good just want to see it for myself, thanks for the info!

@kutsan
Copy link
Contributor

kutsan commented Apr 18, 2018

@ryanoasis For the sake of info, this is what it looks like in MacVim. Removing artifact fix chars solve that problem. I don't think it's needed anymore.

image

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

No branches or pull requests

3 participants