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

Support unusual characters in file and directory names #868

Merged
merged 21 commits into from Oct 25, 2018

Conversation

PhilRunninger
Copy link
Member

@PhilRunninger PhilRunninger commented Aug 4, 2018

Fixes: #680 #492 #469 #424 #391 #217

This PR has the potential to fix a bunch of issues. The problem with these issues is related to parsing the file or directory name from the current line of text. In short, I fixed it by surrounding the filename with non-breaking spaces (ASCII code 160), and using that pair of delimiters to find the filename in the text when needed. I also added syntax highlighting to conceal this character, making it appear as if nothing has been added. The advantages to using this character are:

  1. It is very unlikely to be used in a filename.
  2. If conceal is not compiled into your Vim, the only side effect is that the tree and trailing flags are pushed over by one extra "space".

There may be cases where this doesn't work so well, though, and some improvements will need to be made to this PR. The one I can think of right away is editing in an environment that doesn't support Unicode characters, or whatever the non-breaking space is. ssh comes to mind as a possible trouble spot.

conceal was added in 7.3, but with the if has("conceal") in the syntax file, we should be ok. It will just have the added spacing indicated above.

I would like to enlist the authors of the related issues to test this fully. Areas I'd like them and others to try are:

  • scenarios listed in the issues
  • gvim
  • console vim
  • using the mouse to interact with NERDTree
  • possible side effects caused to other plugins like
    • nerdtree-git-plugin
    • devicons
  • ssh
  • older versions of vim 7.0 - 7.2
  • Windows
  • Linux
  • Mac

Of course, I don't expect everyone to test all these areas, but given enough people we can likely cover them all.

Phil Runninger (mac) added 9 commits July 5, 2018 01:18
I don't know what I was thinking. The delimiter doesn't need to be used
to separate every indicator on the node's text, ie.

Bad:    Tree|GenericFlags|Filename|ExecutableFlag|Link|ReadonlyFlag
Better: Tree GenericFlags|Filename|ExecutableFlag Link ReadonlyFlag

This was unnecessary, given that we're only interested in the filename.
So, just one pair of delimiters is all we need. That greatly simplifies
the _stripMarkup function, and restores a bunch of other statements to
what they already are in the master branch.
@lifecrisis
Copy link
Contributor

Wow, Phil! You've been busy. I'll make sure to take a look at this this week!

Phil Runninger (mac) added 2 commits August 6, 2018 21:50
This is probably the best we can do, especially if some other character
must be used in place of nbsp.
It was allowing 2+ spaces, instead of only 1+.
@PhilRunninger
Copy link
Member Author

@lifecrisis , did you get a chance to review this yet? I'm going to start tagging the issue authors for their help in reviewing this too.

@jsdevel
Copy link

jsdevel commented Aug 17, 2018

So far, doesn't seem to have fixed #555

not-working

@xcambar
Copy link

xcambar commented Aug 20, 2018

It fixes #680 🎉

@jsdevel
Copy link

jsdevel commented Aug 20, 2018

@PhilRunninger fixes #555 !

works

@lifecrisis
Copy link
Contributor

lifecrisis commented Aug 25, 2018

Issues #555 and #680 fail to remain fixed with cascaded directories. Use...

$ mkdir \[foo\]/bar/
$ mkdir \{foo\}/bar/

... as an example. I'll be back to test more when these problems are fixed!

EDIT: To be clear, you can't toggle open and closed the cascades properly with the o mapping.

@lifecrisis lifecrisis removed their request for review August 25, 2018 15:25
Using ':' as a more visible delimiter, when directories are cascaded,
the line appears in NERDTree like so:

▾ :lib/::nerdtree/:

Before this commit, the s:UI._stripMarkup function was leaving the
internal delimiters in place (lib/::nerdtree/). Now they are removed,
resulting in a valid path (lib/nerdtree/).
@PhilRunninger
Copy link
Member Author

@lifecrisis , your observation has been addressed. Take another look.

Thanks.

@lifecrisis
Copy link
Contributor

It seems that the functionality is all there! Awesome!

I still want to take a closer look at the code itself, so I'll try to make it happen later this week. This is a pretty high-impact change!

@lifecrisis
Copy link
Contributor

I'm getting hung up on something small here...

Go to the line of any NERDTree node. Press 0 and then press l several times. You'll see that the l motion will "catch" on the first word of the directory. Any way to avoid this problem while still concealing the characters? Nothing jumps out at me immediately.

@lifecrisis
Copy link
Contributor

lifecrisis commented Sep 16, 2018

I also noticed that, now, I can move the cursor beyond the end of the line. I.e., I can move the cursor to the trailing nbsp character.

I think I have a solution to this...

Instead of relying on concealing the character, just show the first nbsp character and only use the second one if there are trailing flags (by having the process for appending the flags also add the nbsp first).

This would mean that a node would be delimited like [nbsp][file name][eol|[nbsp][link...]]. This would get rid of the change to how the cursor moves around.

Also, just curious, how are we determining that nbsp is even available in the user's context? What is the likelihood that this character will be available? Is it almost certain?

@PhilRunninger
Copy link
Member Author

@lifecrisis , I tried implementing your solution, but I think I'll stick with what's committed already. On filenames and single directories, there are no issues, and I can parse them easily. Here are some examples: (A colon is used in place of the nbsp below.)

:.vim/
:nerdtree/: {nerdtree}
:path.vim
: space_in_first_position.txt
:vimrc: {vimrc}
:foobar: -> /tmp/foobar

However, when dealing with cascaded directories, we have this situation.

:lib/:nerdtree/
:foo/:bar/: {foobar}

There is no easy way to distinguish the intermediate : from a trailing one, and as a result, files in these directories cannot be opened.

I think the cursor's "catching" on the leading nbsp, and moving beyond the node where the trailing nbsp is, are minor issues, as horizontal movement isn't really needed (and probably uncommon) in NERDTree.

Whether nbsp is available in all fonts is a mystery. I have no way of knowing for sure, but I'd guess it fairly likely is. That being said, since it's being concealed anyway (concealing requires 7.3+), it shouldn't really matter how that character is rendered.

@lifecrisis
Copy link
Contributor

lifecrisis commented Oct 18, 2018

Ever thought about just associating each node with the line number it's written on though a Vim dictionary? Then we could just render each node as we please. The dictionary could be updated every time the NERDTree is drawn. Is this crazy? I'd be worried that that process might become slow for large directories.

EDIT: The efficiency issue could possibly be solved by giving each node a unique integer identifier that we can use as a reference.

@PhilRunninger
Copy link
Member Author

I like that idea. It would take some time to get everything right, and performance could be a problem, but we won't know if we don't try. What do you suggest?

  1. Adding line number to the node objects in tree_file_node.vim and tree_dir_node.vim, or
  2. A new separate dictionary/array associating line number to filename, or
  3. Something else

@lifecrisis
Copy link
Contributor

It's a pretty complicated idea... I'll have to think about it for a bit.

@lifecrisis
Copy link
Contributor

I thought about what you wrote above:

There is no easy way to distinguish the intermediate : from a trailing one, and as a result, files
in these directories cannot be opened.

I'm not sure why you need intermediate nbsp characters... the NERDTree directory separator is always the / character. Furthermore, you always know that the trailing nbsp character is the trailing one because it's the last one.

I'm not really following the thinking here. Sorry if I'm just confusing myself...

@PhilRunninger
Copy link
Member Author

Assuming we use the trailing nbsp only when adding the *, {bookmarks}, -> symlink, or [RO] tags,...

The nodes are separate objects in memory, so each dir node would have the leading delimiter.

:lib/
  :nerdtree/

but when they are combined for cascading, they look like this

:lib/:nerdtree/

This is the behavior you noticed in this comment above. (Remember, at that time it would have been :lib/::nerdtree/:.) In that case, it was treating intermediate delimiters as part of the directory name, until I made the fix in e1af7a3.

Getting rid of the leading nbsp and everything before it is easy, but if we blindly get rid of the last nbsp and everything after it, we could end up losing the last folder of this cascaded directory node.

:lib/:nerdtree/ erroneously becomes lib/, while
:lib/:nerdtree/: {bookmark} correctly becomes lib/nerdtree/.

So it's a specific case of cascaded directories and no tags that causes the suggested fix to fail.

I just thought of a way to make this work. The code that combines the parent and child directory nodes can strip the leading nbsp off each child directory. I haven't looked at the code to see if it will work, but I think that's the way to go.

@lifecrisis
Copy link
Contributor

I just thought of a way to make this work. The code that combines the parent and child directory nodes can strip the leading nbsp off each child directory. I haven't looked at the code to see if it will work, but I think that's the way to go.

That's exactly what I meant! I feel like I'm being a bit difficult on this issue, but I'm adamant that this is something we need to get right. It's such a fundamental change, and it would be very useful.

(Nice GIF by the way.)

Perhaps we could go with node ID numbers, associative arrays, etc., in the next version of the NERDTree and just use this for now (you're only affecting a few dozen lines of code here).

* If flags are needed after the node name, then put another delimiter
before them.
* When joining directory nodes for cascaded display, strip off the
delimiter from the child node(s).
* Remove the unnecessary substitution of doubled intermediate
delimiters, since they're not in there anymore.
@PhilRunninger
Copy link
Member Author

I feel like I'm being a bit difficult on this issue, but ...

No worries. I'm glad to have you reviewing this. It results in a much better solution that way. New commits (final ones, I hope) coming soon.

@PhilRunninger
Copy link
Member Author

@lifecrisis , that should just about do it. Take a look and let me know.

PhilRunninger pushed a commit to PhilRunninger/nerdtree-buffer-ops that referenced this pull request Oct 19, 2018
@PhilRunninger PhilRunninger merged commit 91e0f22 into master Oct 25, 2018
@PhilRunninger PhilRunninger deleted the nonstandard-characters branch October 25, 2018 02:41
heni added a commit to heni/settings that referenced this pull request Dec 11, 2018
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.

Cannot open file or directory whose name include [ or ]
4 participants