Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Comments

Tildify path in status bar#171

Merged
damieng merged 1 commit intoatom:masterfrom
lgeiger:tildify-path
Oct 12, 2016
Merged

Tildify path in status bar#171
damieng merged 1 commit intoatom:masterfrom
lgeiger:tildify-path

Conversation

@lgeiger
Copy link
Contributor

@lgeiger lgeiger commented Sep 22, 2016

This will use ~/ for the path to the home directory to be consistent with the path displayed in the title (atom/atom#12753)

Unfortunately one test fails here because fs.tildify normalizes the path and therefor removes the double slashes. How should this be handled?

@thomasjo
Copy link

Unfortunately one test fails here because fs.tildify normalizes the path and therefor removes the double slashes. How should this be handled?

Initial reaction is that it's a bug in fs.tildify, but remote://server:123/folder/remote_file.txt isn't really a path at all, it's a URI — my proposal is that we check whether a "path" is a URI before we pass it onto any path routines. To check whether the path is a URI we can do something super naive like candidatePath?.indexOf('://') > 0 or we can use url.parse and then check if e.g. the protocol property is truthy.

I'm not entirely decided on exactly what we should do in the case of the "path" being a URI, but since the tildify module doesn't support URI-based home directories anyway, I think the best option is to not pass the "path" fs.tildify if we have determined it's actually a URI.

/cc @atom/feedback

@thomasjo
Copy link

Thinking about this some more, since the tildify module doesn't support URIs at all, I think we should patch this in fs.tildify by checking if the given "path" is a URI or not.

@lgeiger
Copy link
Contributor Author

lgeiger commented Sep 25, 2016

@thomasjo Thanks a lot for your feedback. I addressed your comments in atom/fs-plus#33.

@damieng
Copy link
Contributor

damieng commented Oct 12, 2016

Do you want to bump the fs-plus to 2.9.3 ?

This will use ~/ for the path to the home directory
@lgeiger
Copy link
Contributor Author

lgeiger commented Oct 12, 2016

Updated ⬆️

@damieng damieng merged commit 414b458 into atom:master Oct 12, 2016
@damieng
Copy link
Contributor

damieng commented Oct 12, 2016

Thanks for your contribution!

@lgeiger lgeiger deleted the tildify-path branch October 12, 2016 19:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants