Skip to content

Conversation

melvinw
Copy link
Collaborator

@melvinw melvinw commented Aug 4, 2024

A small quality of life improvement for job control. @andychu brought this up in https://oilshell.zulipchat.com/#narrow/stream/121540-oil-discuss/topic/Job.20control.20hard.20to.20understand

[I] osh-0.22.0$ cat
^Z
[PID 489245] Stopped with signal 20
[I] osh-0.22.0$ cat
^Z
[PID 489246] Stopped with signal 20
[I] osh-0.22.0$ cat
^Z
[PID 489247] Stopped with signal 20
[I] osh-0.22.0$ jobs
%1 489245 Stopped [process] cat
%2- 489246 Stopped [process] cat
%3+ 489247 Stopped [process] cat

Copy link
Contributor

@andychu andychu left a comment

Choose a reason for hiding this comment

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

Thanks!

I wonder if we can add a spec test like

$ bin/osh -c 'sleep 0.01 & sleep 0.02 & sleep 0.03 & jobs'
%1 3423 Running [subprog] command.Simple
%2 3424 Running [subprog] command.Simple
%3 3425 Running [subprog] command.Simple

(Although we really need to print code correctly)

...

I also noticed the pipeline case is a bit weird

we are not printing a job num

$ bin/osh -c 'sleep 0.1 | wc -l & jobs'
   3440 Running [subprog] command.Simple

bash and zsh do

$ bash -c 'sleep 0.1 | wc -l & jobs'
[1]+  Running                 sleep 0.1 | wc -l &

core/process.py Outdated
@@ -1777,9 +1777,17 @@ def DisplayJobs(self, style):
# 24510 | sleep 5 &

f = mylib.Stdout()
current, previous = self.GetCurrentAndPreviousJobs()
for job_id, job in iteritems(self.jobs):
# Use the %1 syntax
Copy link
Contributor

Choose a reason for hiding this comment

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

this comment can move to _DisplayJob()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

core/process.py Outdated
for job_id, job in iteritems(self.jobs):
# Use the %1 syntax
job.DisplayJob(job_id, f, style)
extra = ''
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should default to one space so it's aligned?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes! Nice catch

@melvinw
Copy link
Collaborator Author

melvinw commented Aug 5, 2024

Let me see about adding a spec test

@andychu andychu deleted the branch soil-staging October 1, 2024 04:33
@andychu andychu closed this Oct 1, 2024
@andychu
Copy link
Contributor

andychu commented Oct 5, 2024

Hm I think we should land this? Let me look at what happened

@andychu andychu reopened this Oct 5, 2024
@andychu andychu deleted the branch soil-staging October 15, 2024 16:00
@andychu andychu closed this Oct 15, 2024
@andychu andychu reopened this Apr 27, 2025
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.

2 participants