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

Show background jobs in bash and zsh #596

Closed
wants to merge 1 commit into
base: develop
from

Conversation

Projects
None yet
3 participants
@phunehehe
Contributor

phunehehe commented Jul 13, 2013

This is for #595

@Lokaltog

This comment has been minimized.

Member

Lokaltog commented Jul 31, 2013

Thanks! I'll review this later today.

@Lokaltog

This comment has been minimized.

Member

Lokaltog commented Jul 31, 2013

The commit looks allright, but I'd like a couple of additions before I merge it. It would be great if you could add unit tests, and add metavar, etc. parameters to the --jobs argument (similar to the existing command line arguments).

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Nov 18, 2013

len(str.splitlines()) is a poor way to count lines. Use str.count('\n')+1 ("$()" truncates trailing newline hence +1), it has far less overhead.

@phunehehe

This comment has been minimized.

Contributor

phunehehe commented Nov 18, 2013

Hi, I didn't notice that I still have this pull request. I have decided not to use powerline anymore. Perhaps somebody else is interested in getting this merged. I'm closing this, but I will leave the branch in my repo in case someone wants it.

@phunehehe phunehehe closed this Nov 18, 2013

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Nov 18, 2013

I was just about to write something like this when remembered that there is your PR. If you are not maintaining it I will open my own: in any case that note is not the only thing I would have implemented differently.

18.11.13, 11:14, "Hoang Xuan Phu" notifications@github.com":

Hi, I didn't notice that I still have this pull request. I have decided not to use powerline anymore. Perhaps somebody else is interested in getting this merged. Should I leave this open?

Reply to this email directly or view it on GitHub.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Nov 18, 2013

By the way, may I ask you why you decided not to use powerline?

18.11.13, 11:14, "Hoang Xuan Phu" notifications@github.com":

Hi, I didn't notice that I still have this pull request. I have decided not to use powerline anymore. Perhaps somebody else is interested in getting this merged. Should I leave this open?

Reply to this email directly or view it on GitHub.

@phunehehe

This comment has been minimized.

Contributor

phunehehe commented Nov 18, 2013

@ZyX-I I have to deal with a wide range of machines and getting Powerline to work with some of them was problematic (especially on my Raspberry Pi running Arch Linux). I wrote a blog post detailing that https://phunehehe.net/powerline-revisited/. The bottom line is, while Powerline is cool and I introduced it to many of my colleagues, it's not a good fit for me.

@ZyX-I

This comment has been minimized.

Contributor

ZyX-I commented Nov 18, 2013

@phunehehe By the way, the method you use to count jobs (counting number of lines of jobs output) is not correct in zsh:

(zyx-desktop:zyx:~) % echo abc | less

zsh: done       echo abc | 
zsh: suspended  less -M
(zyx-desktop:zyx:~) 1 % jobs
[1]  + done       echo abc | 
       suspended  less -M
(zyx-desktop:zyx:~) 1 % jobs | wc -l
2

(the number before per cent is jobs number as reported by zsh using %j).

ZyX-I added a commit to ZyX-I/powerline that referenced this pull request Nov 18, 2013

Add jobnum segment
Replaces powerline#596. Differences:
- Tests and metavar.
- Uses “jobnum” name in place of “jobs”.
- Does not use subshell for zsh. Also counts jobs correctly in zsh.
- Adds an option to force showing jobnum segment even if there are no jobs.

@ZyX-I ZyX-I referenced this pull request Nov 18, 2013

Merged

Add jobnum segment #712

@phunehehe

This comment has been minimized.

Contributor

phunehehe commented Nov 18, 2013

I think you have just found a bug that I have seen from time to time, but never got around to pin it down :)
Great find! I'll use it in my dotfiles too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment