Add digits editing for numeric options by fasterit · Pull Request #1935 · htop-dev/htop

fasterit · 2026-03-29T17:17:59Z

Also add NumPad +/- keys for increment / decrement in numeric options

Assisted-by: Microsoft Copilot/Claude Sonnet 4.6

Also add NumPad +/- keys for increment / decrement in numeric options

Explorer09 · 2026-03-29T22:11:28Z

OptionItem.h

+void NumberItem_startEditing(NumberItem* this);
+void NumberItem_startEditingFromValue(NumberItem* this);
+void NumberItem_cancelEditing(NumberItem* this);
+bool NumberItem_applyEditing(NumberItem* this);
+bool NumberItem_addChar(NumberItem* this, char c);
+void NumberItem_deleteChar(NumberItem* this);


So many functions being added. I'm becoming skeptical of what the AI is changing here.

Waldorf and Statler, are you doing the code reviews here?

Explorer09 · 2026-03-29T22:16:46Z

OptionItem.c

+         this->editBuffer[0] = '\0';
+         return false;
+      }
+      newValue = (int) round(displayValue / pow(10, this->scale));


Since this->scale is negative in this conditional, why not use multiplication (* pow(10, -this->scale)) for the scale instead of division? And what's the rationale for the rounding direction?

That scale nonsense isn't of my making, cf. 267014c.

On the div: Isn't your mantra to leave such tiny optimization to the compiler?

round gets the nearest integer, seems reasonable, not? I did not even think of alternatives. Just need to clamp the user, that types in 1.52334 for a x.x field.

Note that the rounding mode may be surprising depending on what is (implicitly) set.

@BenBE The FP rounding mode is per-thread, and is not carried over to another executable image loaded by execve(2), so in other words, the rounding mode is in our control. However, I noticed a round(3) call here and the reason for the rounding direction is not given.

The context seems to be that, a binary floating point is used to parse a string with decimals, and then it tries to convert the floating point to a fixed point. This raises a question: why not parse to a fixed point directly?

1.56 gets rounded to 1.6 in the "Update Interval" I consider that most correct. And it was the most easy solution, too.

I though more on the lines of 1.45 vs 1.55, which depending on rounding mode may both be 1.5 (even/odd rounding vs. mathematically correct rounding) instead of 1.5 vs 1.6.

OptionItem.c

Explorer09 · 2026-03-29T22:21:37Z

OptionItem.c

+   int len;
+   /* Format the initial edit buffer using scale for decimal display */
+   if (this->scale < 0) {
+      len = xSnprintf(tmp, sizeof(tmp), "%.*f", -this->scale, pow(10, this->scale) * this->savedValue);


Potential rounding error. It looks like AI didn't care about this.

len is max ten chars, where do you see an issue here?

OptionItem.c

DisplayOptionsPanel.c

Explorer09 · 2026-03-29T22:38:27Z

CRT.h

+# define KEY_PADPLUS    583
+# define KEY_PADMINUS   588


Why these numbers and where are they from?

Because that is what xterm compatible terminals send when you press + / - on the Numpad without Numlock active. If you have some Mac values for that, happy to add.

BenBE

Minor stylistic notes …

BenBE · 2026-03-30T22:52:44Z

DisplayOptionsPanel.c

            }
         }
-         /* fallthrough */
+         /* FALLTHROUGH */


Unrelated change. I think the rest of the code all uses lowercase for this.

I think they should all be uppercase but fine to revert this one and may be we can change them all in one go if you lot agree.

According to GCC documentation the match is made case-insensitive, thus both are fine.

Current distribution on main is:

$ grep -Ri 'fallthr' DisplayOptionsPanel.c: /* fallthrough */ OpenFilesScreen.c: } /* FALLTHRU */ MetersPanel.c: /* else fallthrough */ MetersPanel.c: /* else fallthrough */ ScreensPanel.c: /* FALLTHRU */ ScreensPanel.c: /* FALLTHRU */ ColumnsPanel.c: /* else fallthrough */ ColumnsPanel.c: /* else fallthrough */

-Wimplicit-fallthrough=3 is basically an extension of -Wimplicit-fallthrough=2 with some stricter case matching; but both variants are covered.

To me it is more about the visual clue. I spot FALLTHRU much better than fallthrough.

DisplayOptionsPanel.c

BenBE · 2026-03-30T23:03:44Z

OptionItem.c

+         this->editBuffer[0] = '\0';
+         return false;
+      }
+      newValue = (int) round(displayValue / pow(10, this->scale));


Note that the rounding mode may be surprising depending on what is (implicitly) set.

OptionItem.h

fasterit · 2026-03-31T09:27:40Z

Please squash on merge

Add digits editing for numeric options

2bdbcf1

Also add NumPad +/- keys for increment / decrement in numeric options

fasterit added the enhancement Extension or improvement to existing feature label Mar 29, 2026