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

Fix missing reference (tabId) in windowsSetCwd() #44

Merged
merged 1 commit into from
May 3, 2018
Merged

Fix missing reference (tabId) in windowsSetCwd() #44

merged 1 commit into from
May 3, 2018

Conversation

asyncore
Copy link
Contributor

Hi,
Not sure if this is the best fix but this change does the job.
The body of windowsSetCwd has been copied from index.js. However the signature has changed, and tabId is no longer passed as an argument. Since the caller already sets curTabId as action.uid, I propose a fix to replace missing tabId with action.uid, although it also makes sense to remove the tabId === curTabId condition altogether, because now it will always be true.

The body of windowsSetCwd has been copied from index.js. However the signature has changed, and tabId is no longer passed as an argument. Since the caller already sets curTabId as action.uid, I propose a fix to replace missing tabId with action.uid, although it also makes sense to remove the `tabId === curTabId` condition altogether, because now it will always be true.
@reevik
Copy link

reevik commented Apr 26, 2018

thanks!

@danyfoo
Copy link

danyfoo commented Apr 30, 2018

@bagdemir Is any posibility to merge this commit to fix this bug?

@panmona
Copy link

panmona commented May 3, 2018

@hharnisc or @bagdemir -> Can you merge this PR? Would be greatly appreciated!

@hharnisc
Copy link
Owner

hharnisc commented May 3, 2018

Hey everyone 👋

So sorry for the delay here, I've been away on vacation 🌴 Going to merge this fix and publish a new release now.

It's become clear this project needs another maintainer, specifically one that lives on windows. Would anyone be up for that? I rely on this for Linux (OS X is same code path for now), so I can promise that will work.

@hharnisc hharnisc merged commit 245517d into hharnisc:master May 3, 2018
@hharnisc
Copy link
Owner

hharnisc commented May 3, 2018

Published as version 1.2.3

@hharnisc
Copy link
Owner

hharnisc commented May 3, 2018

Thank you for the fix @asyncore 🙌

@panmona
Copy link

panmona commented May 3, 2018

Wow this was fast. Thanks @hharnisc! Works well for me now.

@kmccmk9
Copy link

kmccmk9 commented May 5, 2018

@hharnisc I use Windows primarily and use hyper everyday. I can do maintenance and pull request checking to make sure it works under Windows.

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.

6 participants