Add user-configurable distribution path prefixes (implements #822) #1961
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,6 +76,27 @@ static void NumberItem_display(const Object* cast, RichString* out) { | |
| RichString_appendWide(out, CRT_colors[CHECK_TEXT], this->super.text); | ||
| } | ||
|
|
||
| static void StringItem_display(const Object* cast, RichString* out) { | ||
| const StringItem* this = (const StringItem*)cast; | ||
| int labelAttr = CRT_colors[CHECK_TEXT]; | ||
| int boxAttr = CRT_colors[CHECK_BOX]; | ||
| int valAttr = this->valid ? CRT_colors[CHECK_MARK] : CRT_colors[FAILED_READ]; | ||
|
Contributor
There was a problem hiding this comment. What does the validate function inside a StringItem do? And what would happen (or should happen) if the validation fails?
Author
There was a problem hiding this comment. Currently its a placeholder. There was the idea of validating if the input path is a valid path in #822 but I did not implement any validation.
Contributor
There was a problem hiding this comment. And what should be done if the "validation" fails?
Author
There was a problem hiding this comment. The input should be displayed in red as visual feedback.
Contributor
There was a problem hiding this comment. The "red" visual feedback would be not good enough if you have a list of path prefixes to edit. Also, I saw no code the "discard" directories that don't exist or are exact duplicates of previously specified paths. |
||
|
|
||
| RichString_writeAscii(out, boxAttr, "["); | ||
| if (this->editing) { | ||
| RichString_appendAscii(out, valAttr, this->editor.buffer); | ||
| } else { | ||
| const char* val = (this->ref && *this->ref) ? *this->ref : NULL; | ||
| if (val) { | ||
| RichString_appendAscii(out, valAttr, val); | ||
|
Contributor
There was a problem hiding this comment. Don't use |
||
| } else { | ||
| RichString_appendAscii(out, CRT_colors[PROCESS_SHADOW], "(empty)"); | ||
| } | ||
| } | ||
| RichString_appendAscii(out, boxAttr, "] "); | ||
| RichString_appendWide(out, labelAttr, this->super.text); | ||
| } | ||
|
|
||
| const OptionItemClass OptionItem_class = { | ||
| .super = { | ||
| .extends = Class(Object), | ||
|
|
@@ -112,6 +133,15 @@ const OptionItemClass NumberItem_class = { | |
| .kind = OPTION_ITEM_NUMBER | ||
| }; | ||
|
|
||
| const OptionItemClass StringItem_class = { | ||
| .super = { | ||
| .extends = Class(OptionItem), | ||
| .delete = OptionItem_delete, | ||
| .display = StringItem_display | ||
| }, | ||
| .kind = OPTION_ITEM_STRING | ||
| }; | ||
|
|
||
| TextItem* TextItem_new(const char* text) { | ||
| TextItem* this = AllocThis(TextItem); | ||
| this->super.text = xStrdup(text); | ||
|
|
@@ -323,3 +353,40 @@ void NumberItem_deleteChar(NumberItem* this) { | |
| this->editBuffer[this->editLen] = '\0'; | ||
| } | ||
| } | ||
| StringItem* StringItem_newByRef(const char* text, char** ref, bool (*validate)(const char* text)) { | ||
|
Contributor
There was a problem hiding this comment. typedef bool (*ValidateStringFn)(const char* str, size_t len, void* context);
Author
There was a problem hiding this comment. Will consolidate into a typedef. Will use your suggested signature with len and context if the feature moves forward. |
||
| StringItem* this = AllocThis(StringItem); | ||
| this->super.text = xStrdup(text); | ||
| this->ref = ref; | ||
|
Contributor
There was a problem hiding this comment. What does
Author
There was a problem hiding this comment.
|
||
| this->editing = false; | ||
| this->valid = true; | ||
| this->validate = validate; | ||
| LineEditor_init(&this->editor); | ||
| if (ref && *ref) | ||
| LineEditor_setText(&this->editor, *ref); | ||
| return this; | ||
| } | ||
|
|
||
| void StringItem_startEditing(StringItem* this) { | ||
| this->editing = true; | ||
| LineEditor_setText(&this->editor, (this->ref && *this->ref) ? *this->ref : ""); | ||
| } | ||
|
|
||
| void StringItem_cancelEditing(StringItem* this) { | ||
| this->editing = false; | ||
| LineEditor_setText(&this->editor, (this->ref && *this->ref) ? *this->ref : ""); | ||
| } | ||
|
|
||
| bool StringItem_applyEditing(StringItem* this) { | ||
| this->editing = false; | ||
| const char* text = this->editor.buffer; | ||
| if (this->validate && !this->validate(text)) { | ||
| this->valid = false; | ||
| return false; | ||
| } | ||
| this->valid = true; | ||
| if (this->ref) { | ||
| free(*this->ref); | ||
| *this->ref = (*text != '\0') ? xStrdup(text) : NULL; | ||
| } | ||
| return true; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,6 +174,28 @@ static inline char* stpcpyWithNewlineConversion(char* dstStr, const char* srcStr | |
| return dstStr; | ||
| } | ||
|
|
||
| static size_t findDistPrefixLength(const char* str, const Settings* settings) { | ||
| static const char* const builtinPrefixes = | ||
| "/bin/:/sbin/:/lib/:/lib32/:/lib64/:/libx32/:" | ||
| "/usr/bin/:/usr/sbin/:/usr/lib/:/usr/lib32/:/usr/lib64/:/usr/libx32/:" | ||
| "/usr/libexec/:/usr/local/bin/:/usr/local/lib/:/usr/local/sbin/:" | ||
| "/nix/store/:/run/current-system/"; | ||
|
|
||
| const char* list = (settings->distPathPrefixes && settings->distPathPrefixes[0] != '\0') | ||
| ? settings->distPathPrefixes | ||
| : builtinPrefixes; | ||
|
|
||
| const char* token = list; | ||
| while (*token) { | ||
| const char* end = String_strchrnul(token, ':'); | ||
| size_t len = end - token; | ||
| if (strncmp(str, token, len) == 0) | ||
| return len; | ||
| token = *end ? end + 1 : end; | ||
| } | ||
| return 0; | ||
|
Contributor
There was a problem hiding this comment. There's still a performance concern with this new function. The old code had a switch-case that could make the lookup slightly faster, but now we're forced to perform a linear search... I wish a slightly faster data structure such as a trie.
Author
There was a problem hiding this comment. In this specific case implementing a trie might be overkill given the small number of prefixes. A full implementation would add significant complexity for what I'd expect to be a minimal performance gain.
Contributor
There was a problem hiding this comment. I dont know. If it's me I'd implement it. Otherwise I don't like to trade a performance loss for a feature that few people use. |
||
| } | ||
|
|
||
| /* | ||
| * This function makes the merged Command string. It also stores the offsets of the | ||
| * basename, comm w.r.t the merged Command string - these offsets will be used by | ||
|
|
@@ -256,51 +278,10 @@ void Process_makeCommandStr(Process* this, const Settings* settings) { | |
|
|
||
| #define CHECK_AND_MARK_DIST_PATH_PREFIXES(str_) \ | ||
| do { \ | ||
| size_t plen = findDistPrefixLength(str_, settings); \ | ||
| if (plen > 0) { \ | ||
| WRITE_HIGHLIGHT(0, plen, CRT_colors[PROCESS_SHADOW], \ | ||
| CMDLINE_HIGHLIGHT_FLAG_PREFIXDIR); \ | ||
| } \ | ||
| } while (0) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RTTI (run-time type information). This is a smell of bad object/class modeling. Unfortunately I don't have any idea to fix it for now.