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

Tab titles now show cwd (#2907) #3015

Conversation

El-Dringo-Brannde
Copy link
Contributor

@El-Dringo-Brannde El-Dringo-Brannde commented May 14, 2018

Closes #2907 #2886 #1188

This PR is ready to be merged, ran dist build and it works all the same on my Mac.

However, I don't have a PC, or a VM of a PC to test the windows pathing on. Theoretically... It should work as I have kindly borrowed the same code that gets CWD from hyper-statusline

lib/utils/fs.js Outdated
if (path) cwd = path[0];
}
} else {
cwd = execSync(`lsof -p ${pid} | awk '$4=="cwd"' | tr -s ' ' | cut -d ' ' -f9-`, {encoding: 'utf8'});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the best way to get the current CWD? 🤔 I wonder if there is an async way of doing it so we don't lock up the app...

Copy link
Contributor Author

@El-Dringo-Brannde El-Dringo-Brannde May 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@albinekb There are async methods of running child_processes, but I thought in Redux making stuff async is frowned upon (still learning, I've heard of thunks being used for async actions).

That's why I only have this function run when the user hits the enter key. I have been using my own custom dist from my fork and there seems to be no discernible performance impact.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be executed each time that '\n' is output in term. Do not execute yes in your fork 😁

@Stanzilla Stanzilla added the 🎨 Type: Enhancement Issue or PR is an enhancement request/proposal for Hyper label May 16, 2018
@Stanzilla Stanzilla requested a review from chabou May 16, 2018 14:08
@chabou
Copy link
Collaborator

chabou commented May 17, 2018

This is not the right approach.
Performances will be heavily impacted.

BTW I'm using oh-my-zsh and my tab title shows my cwd.
But by default, I would have preferred to have current running command. I think that it was the case in hyper 1.X

But feel free to make a plugin with your work 🙏

Copy link
Collaborator

@chabou chabou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the right approach.

@El-Dringo-Brannde
Copy link
Contributor Author

If I had it read the enter key press to display the current running command would the PR have a chance of being accepted?

@xiaket xiaket mentioned this pull request May 17, 2018
2 tasks
@El-Dringo-Brannde
Copy link
Contributor Author

El-Dringo-Brannde commented May 27, 2018

@chabou @albinekb Can I get a re-review, I had it watch on enter key presses only, and it is now asynchronous and non-blocking. Or, at the very least, a hint on what would be acceptable features for a PR? I'm trying to get something merged for my final project assignment in my OSS class.

@Stanzilla
Copy link
Collaborator

@El-Dringo-Brannde willing to pick this up again?

@Stanzilla Stanzilla closed this Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 Type: Enhancement Issue or PR is an enhancement request/proposal for Hyper WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revisit the idea of rename tabs
5 participants