Skip to content

Feature/status line errors #1879

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.

Sign up for GitHub

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

Open
DeltachangeOG wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Feature/status line errors #1879

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f2a43a7
Framework for warning display above function bar
DeltachangeOG Jan 25, 2026
90edaa4
Dismiss error warning on user input
DeltachangeOG Jan 25, 2026
0bc5677
show FunctionBar warning on sendSignal failure
DeltachangeOG Jan 25, 2026
12fbcd2
Normalize timeoutMs to monotonic time uint64_t
DeltachangeOG Jan 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions Action.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,18 +519,25 @@ static Htop_Reaction actionKill(State* st) {

static int preSelectedSignal = SIGNALSPANEL_INITSELECTEDSIGNAL;

sendSignalContext ctx;

Panel* signalsPanel = SignalsPanel_new(preSelectedSignal);
const ListItem* sgn = (ListItem*) Action_pickFromVector(st, signalsPanel, 14, true);
if (sgn && sgn->key != 0) {
preSelectedSignal = sgn->key;
ctx.sgn = sgn->key;
ctx.savedErrno = 0;
Panel_setHeader((Panel*)st->mainPanel, "Sending...");
Panel_draw((Panel*)st->mainPanel, false, true, true, State_hideFunctionBar(st));
refresh();
bool ok = MainPanel_foreachRow(st->mainPanel, Process_rowSendSignal, (Arg) { .i = sgn->key }, NULL);
if (!ok) {
bool ok = MainPanel_foreachRow(st->mainPanel, Process_rowSendSignal, (Arg) { .v = &ctx }, NULL);
(void) ok;
if (ctx.savedErrno != 0) {
beep();
FunctionBar_setWarning(strerror(ctx.savedErrno), 0, true);
}
else {
napms(500);
}
napms(500);
}
Panel_delete((Object*)signalsPanel);

Expand Down
57 changes: 57 additions & 0 deletions FunctionBar.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ in the source distribution for its full text.
#include "config.h" // IWYU pragma: keep

#include "FunctionBar.h"
#include "Platform.h"

#include <assert.h>
#include <stdlib.h>
Expand All @@ -30,6 +31,13 @@ static const int FunctionBar_EnterEscEvents[] = {13, 27};

static int currentLen = 0;

static char warningBuf[256];
static bool warningActive;
static bool warningNeedsCleared;
static bool warningDismissOnKeypress;
static uint64_t warningExpiresAtMs;
static const uint32_t warningDefaultTimeoutMs = 1500;

FunctionBar* FunctionBar_newEnterEsc(const char* enter, const char* esc) {
const char* functions[FUNCTIONBAR_MAXEVENTS + 1] = {enter, esc, NULL};
return FunctionBar_new(functions, FunctionBar_EnterEscKeys, FunctionBar_EnterEscEvents);
Expand Down Expand Up @@ -80,6 +88,20 @@ void FunctionBar_delete(FunctionBar* this) {
free(this);
}

void FunctionBar_clearWarning(void) {
warningActive = false;
warningBuf[0] = '\0';
warningDismissOnKeypress = false;
warningExpiresAtMs = 0;
warningNeedsCleared = true;
}

void FunctionBar_inputEvent(void) {
if (warningDismissOnKeypress) {
FunctionBar_clearWarning();
}
}

void FunctionBar_setLabel(FunctionBar* this, int event, const char* text) {
for (size_t i = 0; i < this->size; i++) {
if (this->events[i] == event) {
Expand All @@ -90,12 +112,47 @@ void FunctionBar_setLabel(FunctionBar* this, int event, const char* text) {
}
}

void FunctionBar_setWarning(const char* msg, uint32_t timeoutMs, bool dismissOnKeypress) {
Comment thread
DeltachangeOG marked this conversation as resolved.
Outdated
if (msg == NULL || msg[0] == '\0') {
FunctionBar_clearWarning();
return;
}
snprintf(warningBuf, sizeof(warningBuf), "%s", msg);
warningActive = true;
warningDismissOnKeypress = dismissOnKeypress;
uint64_t now = 0;
Platform_gettime_monotonic(&now);
if (timeoutMs == 0) {
warningExpiresAtMs = now + warningDefaultTimeoutMs;
}
else {
warningExpiresAtMs = now + timeoutMs;
}
}

int FunctionBar_draw(const FunctionBar* this) {
return FunctionBar_drawExtra(this, NULL, -1, false);
}

int FunctionBar_drawExtra(const FunctionBar* this, const char* buffer, int attr, bool setCursor) {
int cursorX = 0;
if (warningActive) {
uint64_t now = 0;
Platform_gettime_monotonic(&now);
if (now >= warningExpiresAtMs) {
FunctionBar_clearWarning();
}
}
if (warningNeedsCleared && LINES > 1) {
attrset(CRT_colors[RESET_COLOR]);
mvhline(LINES - 2, 0, ' ', COLS);
warningNeedsCleared = false;
}
if (warningActive && LINES > 1) {
attrset(CRT_colors[FAILED_READ]);
mvhline(LINES - 2, 0, ' ', COLS);
mvaddstr(LINES - 2, 0, warningBuf);
}
attrset(CRT_colors[FUNCTION_BAR]);
mvhline(LINES - 1, 0, ' ', COLS);
int x = 0;
Expand Down
6 changes: 6 additions & 0 deletions FunctionBar.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ void FunctionBar_delete(FunctionBar* this);

void FunctionBar_setLabel(FunctionBar* this, int event, const char* text);

void FunctionBar_setWarning(const char* msg, uint32_t timeoutMs, bool dismissOnKeypress);

void FunctionBar_clearWarning(void);

void FunctionBar_inputEvent(void);

int FunctionBar_draw(const FunctionBar* this);

int FunctionBar_drawExtra(const FunctionBar* this, const char* buffer, int attr, bool setCursor);
Expand Down
17 changes: 13 additions & 4 deletions Process.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ in the source distribution for its full text.
#include <string.h>
#include <time.h>
#include <sys/resource.h>
#include <errno.h>

#include "CRT.h"
#include "Hashtable.h"
Expand Down Expand Up @@ -901,14 +902,22 @@ bool Process_rowChangePriorityBy(Row* super, Arg delta) {
return Process_setPriority(this, (int)this->nice + delta.i);
}

static bool Process_sendSignal(Process* this, Arg sgn) {
return kill(Process_getPid(this), sgn.i) == 0;
static bool Process_sendSignal(Process* this, sendSignalContext* ctx) {
if (kill(Process_getPid(this), ctx->sgn ) != 0) {
int e = errno;
if (e != ESRCH) {

@Explorer09 Explorer09 Jan 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why filter this specific error code?

@DeltachangeOG DeltachangeOG Jan 30, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ESRCH is often a silent failure in other projects/management scripting. Since no process found means the process exited before the kill command sent, the process ended, just not from kill. Signaling an error could cause confusion to users and add noise that overshadows more useful errors. This is a UX decision for usability and clarity as ESRCH errors do not facilitate any user actions, and processes exiting is normal on Unix systems.

@Explorer09 Explorer09 Jan 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But you stop the loop instead of continue. This doesn't look like you are ignoring the error, but like you are hiding the error deliberately from user.

@DeltachangeOG DeltachangeOG Jan 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

To the end user the ESRCH/successful kill are the same, so I break the loop to return true to the caller. If it is any other error then it gets displayed on the error bar.

Edit: I misspoke. We treat ESRCH as failure, but treat it as one not worth having an error displayed.

If htop wants all errors surfaced with no filtering I can do that.

@Explorer09 Explorer09 Jan 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

htop can kill multiple processes in a loop. There's a tag feature (Space key) to select multiple processes, then press the k key, and it can kill multiple processes.

@Explorer09 Explorer09 Jan 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@DeltachangeOG I admit I didn't test myself, but, since htop can kill multiple processes at once, shouldn't the status bar messages show a list of processes that failed to kill? The message could look like:

Failed to kill process 1234: Operation not permitted
Failed to kill process 1240: Operation not permitted
Failed to kill process 5678: Operation not permitted
Failed to kill process <PID>: <message from strerror(errno)>

@DeltachangeOG DeltachangeOG Jan 30, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ESRCH currently returns false, that behavior is maintained. We could not filter out ESRCH, and maintain the return of False on any failure, but with an error message

or

Treat ESRCH as success due to the end user goal being completed either by kill or the process exiting for other reasons.

I lean toward treating ESRCH as success here since from the user’s perspective the process is already gone (same end state as a successful kill). Please let me know if you have additional thoughts.

@Explorer09 Explorer09 Jan 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The return value for a loop callback function should indicate whether the loop should continue. The general idea (if the return type wasn't boolean) is that no error = continre loop, and error = break loop and return error.
That's what I interpret this function. It's used as a callback in a loop and so should have this semantic.

@DeltachangeOG DeltachangeOG Jan 30, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I didn't want to alter the structure that was already in place, so I maintained current behavior of the call, and just added error details when there was a failure.

I can queue errors, or display multiple errors, or we can have a hierarchy of errors and only display the "top" error.

If we want failures to break the loop and display an error then I would vote we treat ESRCH as success for those semantics.

I tried to implement this feature with as little impact on current behavior as possible, however I am happy to implement whatever direction the maintainers want for this.

@Explorer09 Explorer09 Jan 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know what the maintainers would decide, but I personally don't like having a sendSignalContext structure in a loop just to output error codes. I didn't have a good suggestion for an alternative yet.

ctx->savedErrno = e;
}
return false;
}
return true;
}

bool Process_rowSendSignal(Row* super, Arg sgn) {
bool Process_rowSendSignal(Row* super, Arg arg) {
Process* this = (Process*) super;
assert(Object_isA((const Object*) this, (const ObjectClass*) &Process_class));
return Process_sendSignal(this, sgn);
sendSignalContext* ctx = (sendSignalContext*) arg.v;
return Process_sendSignal(this, ctx);
}

int Process_compare(const void* v1, const void* v2) {
Expand Down
5 changes: 5 additions & 0 deletions Process.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,11 @@ typedef struct Process_ {
ProcessMergedCommand mergedCommand;
} Process;

typedef struct sendSignalContext_ {
int sgn;
int savedErrno;
} sendSignalContext;

typedef struct ProcessFieldData_ {
/* Name (displayed in setup menu) */
const char* name;
Expand Down
2 changes: 2 additions & 0 deletions ScreenManager.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,8 @@ void ScreenManager_run(ScreenManager* this, Panel** lastFocus, int* lastKey, con
continue;
}

FunctionBar_inputEvent();

switch (ch) {
case KEY_ALT('H'): ch = KEY_LEFT; break;
case KEY_ALT('J'): ch = KEY_DOWN; break;
Expand Down
Loading