Delete multiple queues with a single cmd #727
Conversation
p1gp1g
force-pushed
the
delete-multiple-queues
branch
2 times, most recently
from
f743f1f
to
00505ca
Compare
| } | ||
| return ink > inl | ||
| }) |
There was a problem hiding this comment.
I think we should do a pre-flight sanity check, and bail out (printing correct error message) if index isn't found or parameter is bad. Not bailing out and not ignoring all entries will leave the user to an "unknown" state, which may cause them to retry the exact same command, minus the erraneous entry.
Example:
> queueshow
Queued jobs:
[0] : https://ffuf.io.fi/FUZZ (active job)
[1] : https://ffuf.io.fi/test1/FUZZ
[2] : https://ffuf.io.fi/another/FUZZ
[3] : https://ffuf.io.fi/job/FUZZ
> queuedel 1 word
[WARN] Not a number: word
> queuedel 1 2
User would expect jobs [1] and [2] to be deleted, but in reality the first command would have deleted job [1], moving the rest up an index, and the end result would be deletion of all three subsequent jobs: [1], [2], [3].
There was a problem hiding this comment.
That's the current behavior, all non-number input has that warning
There was a problem hiding this comment.
For instance:
> qd 111 a
[WARN] No such queued job. Use "queueshow" to list the jobs in queue
[WARN] Not a number: a
I still have to improve the "no such queued job"
There was a problem hiding this comment.
It currently just shows the warning, but will delete other entries in the command. In case your example above would have had a valid queue entry, that valid entry would have been deleted.
While there would be a "Job XX successfully deleted!" - output, the thing that the user typically concentrates is the warning message.
There was a problem hiding this comment.
Doing a more realistic test :
Queued jobs:
[0] : https://[REDACTED]/FUZZ (active job)
[1] : https://[REDACTED]/2/FUZZ
[2] : https://[REDACTED]/2016/FUZZ
[3] : https://[REDACTED]/2017/FUZZ
[4] : https://[REDACTED]/2019/FUZZ
[5] : https://[REDACTED]/2018/FUZZ
> qd 2 a 3
[INFO] Job successfully deleted!
[INFO] Job successfully deleted!
[WARN] Not a number: a
> q
Queued jobs:
[0] : https://[REDACTED]/FUZZ (active job)
[1] : https://[REDACTED]/2/FUZZ
[2] : https://[REDACTED]/2019/FUZZ
[3] : https://[REDACTED]/2018/FUZZ
> qd 2 a b 3 c
[INFO] Job successfully deleted!
[INFO] Job successfully deleted!
[WARN] Not a number: a
[WARN] Not a number: b
[WARN] Not a number: c
> q
Queued jobs:
[0] : https://[REDACTED]/FUZZ (active job)
[1] : https://[REDACTED]/2/FUZZ
There was a problem hiding this comment.
Non-number inputs are always at the end of the slice
There was a problem hiding this comment.
Note that "Job successfully deleted" is now "Job x successfully deleted"
pkg/interactive/termhandler.go
Outdated
| i.Job.Output.Warning("Cannot delete the currently running job. Use \"queueskip\" to advance to the next one") | ||
| } else { | ||
| i.Job.DeleteQueueItem(index) | ||
| i.Job.Output.Info("Job successfully deleted!") |
There was a problem hiding this comment.
It might be "jobs" in some cases after merging this PR ;)
| i.Job.DeleteQueueItem(index) | ||
| i.Job.Output.Info("Job successfully deleted!") | ||
| func (i *interactive) deleteQueue(in []string) { | ||
| sort.Slice(in, func(k, l int) bool { |
There was a problem hiding this comment.
Add a comment to tell that a reverse sort based on Atoi values is what's accomplished here to keep the code readable
|
Oh, and we need to change the interactive mode help text (and a |
p1gp1g
force-pushed
the
delete-multiple-queues
branch
from
00505ca
to
7bc61b6
Compare
|
I've force-pushed the changes |
Description
We often have many queue to delete. This allows to run
queuedel 1 2 3 4.Additonally
CONTRIBUTORS.md.The file should be alphabetically ordered.
CHANGELOG.md