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

Update setPath to work with custom files #277

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hansonw
Copy link
Contributor

@hansonw hansonw commented Nov 1, 2017

Currently, setPath creates a new File object. If we already have a custom file object we should just call setPath on it, along with the other setup to update the subscriptions and send the event.

Contributed under CC0.

@hansonw
Copy link
Contributor Author

hansonw commented Nov 8, 2017

@maxbrunsfeld could you take a look at this when you get a chance? much appreciated :)

@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented Nov 8, 2017

Just so I understand - why is this preferable to just calling setFile with a new instance of the same file class? Up until now, I had envisioned setPath(filePath) as being a convenient shorthand for
setFile(new pathwatcher.File(filePath)). I'm open to changing that behavior though.

@hansonw
Copy link
Contributor Author

hansonw commented Nov 8, 2017

Just convenience I guess - for example, creating remote files in Nuclide requires a fair amount of dependencies. It's a bit roundabout to have to create a new file when File implementations are able to change their own paths without much of an issue. The current behavior also means that we've effectively had to "blacklist" TextEditor.setPath from the Nuclide codebase, which seems a little odd.

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.

None yet

2 participants