Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Would love to contribute to this project but I'm confused by the PID usage #81

Open
rfgamaral opened this issue May 25, 2018 · 4 comments

Comments

@rfgamaral
Copy link

I use Hyper with WSL and this plugin doesn't work with it (see #75). I'm trying to fix that but I come across one thing that is confusing me and I'm not sure how to proceed until I understand what is going on.

Please take a look at:

Why do we need to get the PID on SESSION_SET_XTERM_TITLE and then do nothing with it? I mean:

  • For SESSION_ADD we get the new session PID and invoke setCwd with that PID, makes sense.
  • For SESSION_SET_ACTIVE we get the active session PID (we could've switched tabs/panes) and invoke setCwd with that PID, makes sense.
  • For SESSION_ADD_DATA we don't get a new PID because it will be the same one as read before either by SESSION_ADD or SESSION_SET_ACTIVE and then invoke setCwd with the current saved PID, makes sense.
  • For SESSION_SET_XTERM_TITLE why do we need to get the active session PID? Especially when we don't even invoke setCwd with that PID... Either way, won't the PID on this event be the exact same one as read before either by SESSION_ADD or SESSION_SET_ACTIVE?

So, is handling SESSION_SET_XTERM_TITLE really needed? Do we really need to read the PID here? If so, why?

Would really like to understand this as I'm not sure how to proceed to fix #75 without understanding this bit of the code base.

@j-f1
Copy link

j-f1 commented May 25, 2018

If it’s possible to fetch the PID in SESSION_ADD_DATA, then the pid global and the SESSION_SET_XTERM_TITLE section seem to be unnecessary.

@rfgamaral
Copy link
Author

@j-f1 But why do we even need to fetch the PID in SESSION_ADD_DATA? The one we currently have when that event is dispatch should be the correct one, we don't need to fetch it again. What am I missing?

@rfgamaral
Copy link
Author

@henrikdahl Could you provide some input here please?

@j-f1
Copy link

j-f1 commented Sep 3, 2018

Feel free to open a PR on my fork. I’ll be happy to review and get the changes published!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants