From e852b7e1828d86880c637535b80d720e120a4622 Mon Sep 17 00:00:00 2001 From: Chris Punches Date: Mon, 16 Mar 2026 03:21:25 -0400 Subject: [PATCH] Add unit validation with red highlighting and status bar feedback Units that cannot be executed are shown in red on both the Units and Plans tabs. Validation checks target existence/executability, shell definition lookup, working directory existence, rectifier existence/executability, and environment file existence. The unit properties dialog applies live validation with GTK error styling on each field and reports issues to the status bar as the user edits. Selecting a unit or task re-runs validation so the status bar always reflects the selected item. Also separates plan dirty state from unit editor dirty state so editing unit properties no longer marks the plan as unsaved. --- src/models/project.cpp | 68 ++++++++++++++++++ src/models/project.h | 1 + src/util/unit_properties_dialog.cpp | 105 +++++++++++++++++++++++++++- src/views/main_window.cpp | 11 +-- src/views/plan_view.cpp | 68 ++++++++++++------ src/views/plan_view.h | 1 + src/views/shells_view.cpp | 11 ++- src/views/unit_editor.cpp | 22 +++--- src/views/unit_editor.h | 8 +-- src/views/units_view.cpp | 32 +++++++-- 10 files changed, 274 insertions(+), 53 deletions(-) diff --git a/src/models/project.cpp b/src/models/project.cpp index c7a9e50..480be53 100644 --- a/src/models/project.cpp +++ b/src/models/project.cpp @@ -18,6 +18,7 @@ #include "models/project.h" #include +#include namespace grex { namespace fs = std::filesystem; @@ -50,6 +51,73 @@ fs::path Project::resolved_shells_path() const { return root / sp; } +bool Project::check_unit_valid(const Unit& u) { + namespace fs = std::filesystem; + + if (u.target.empty()) { + report_status("Unit '" + u.name + "': target is not defined"); + return false; + } + if (!fs::exists(u.target)) { + report_status("Unit '" + u.name + "': target does not exist: " + u.target); + return false; + } + if (access(u.target.c_str(), X_OK) != 0) { + report_status("Unit '" + u.name + "': target is not executable: " + u.target); + return false; + } + + if (u.is_shell_command) { + bool found = false; + for (auto& s : all_shells()) { + if (s.name == u.shell_definition) { found = true; break; } + } + if (!found) { + report_status("Unit '" + u.name + "': shell definition not found: " + u.shell_definition); + return false; + } + } + + if (u.set_working_directory) { + if (u.working_directory.empty()) { + report_status("Unit '" + u.name + "': working directory is not defined"); + return false; + } + if (!fs::is_directory(u.working_directory)) { + report_status("Unit '" + u.name + "': working directory does not exist: " + u.working_directory); + return false; + } + } + + if (u.rectify) { + if (u.rectifier.empty()) { + report_status("Unit '" + u.name + "': rectifier is not defined"); + return false; + } + if (!fs::exists(u.rectifier)) { + report_status("Unit '" + u.name + "': rectifier does not exist: " + u.rectifier); + return false; + } + if (access(u.rectifier.c_str(), X_OK) != 0) { + report_status("Unit '" + u.name + "': rectifier is not executable: " + u.rectifier); + return false; + } + } + + if (u.supply_environment) { + if (u.environment.empty()) { + report_status("Unit '" + u.name + "': environment file is not defined"); + return false; + } + if (!fs::exists(u.environment)) { + report_status("Unit '" + u.name + "': environment file does not exist: " + u.environment); + return false; + } + } + + return true; +} + Unit* Project::find_unit(const std::string& unit_name) { for (auto& uf : unit_files) { auto* u = uf.find_unit(unit_name); diff --git a/src/models/project.h b/src/models/project.h index b99e249..2f7b1ea 100644 --- a/src/models/project.h +++ b/src/models/project.h @@ -50,6 +50,7 @@ public: std::filesystem::path resolved_units_dir() const; std::filesystem::path resolved_shells_path() const; + bool check_unit_valid(const Unit& u); Unit* find_unit(const std::string& unit_name); UnitFile* find_unit_file(const std::string& unit_name); bool is_unit_name_taken(const std::string& name, const Unit* exclude = nullptr) const; diff --git a/src/util/unit_properties_dialog.cpp b/src/util/unit_properties_dialog.cpp index 0d48b1b..d0f15ef 100644 --- a/src/util/unit_properties_dialog.cpp +++ b/src/util/unit_properties_dialog.cpp @@ -19,6 +19,7 @@ #include "util/unit_properties_dialog.h" #include #include +#include namespace grex { @@ -93,14 +94,17 @@ struct DialogState { }; static void update_sensitivity(DialogState* s); +static void validate_fields(DialogState* s); static gboolean dlg_switch_state_set_cb(GtkSwitch* sw, gboolean new_state, gpointer data) { auto* b = static_cast(data); gtk_switch_set_state(sw, new_state); if (b->state) { *b->target = new_state; - if (!b->state->loading) + if (!b->state->loading) { update_sensitivity(b->state); + validate_fields(b->state); + } } return TRUE; } @@ -361,6 +365,102 @@ static void update_sensitivity(DialogState* s) { show(active && s->working_copy.supply_environment, {s->label_environment, s->box_environment}); } +static void validate_fields(DialogState* s) { + if (s->loading) return; + + auto& u = s->working_copy; + namespace fs = std::filesystem; + + auto set_valid = [](GtkWidget* w, bool valid) { + if (valid) + gtk_widget_remove_css_class(w, "error"); + else + gtk_widget_add_css_class(w, "error"); + }; + + std::string error_msg; + + // Target: must be defined, exist, and be executable + { + bool valid = true; + if (u.target.empty()) { + valid = false; + if (error_msg.empty()) error_msg = "Target is not defined"; + } else if (!fs::exists(u.target)) { + valid = false; + if (error_msg.empty()) error_msg = "Target does not exist: " + u.target; + } else if (access(u.target.c_str(), X_OK) != 0) { + valid = false; + if (error_msg.empty()) error_msg = "Target is not executable: " + u.target; + } + set_valid(s->entry_target, valid); + } + + // Shell definition: if shell command, must reference a known shell + if (u.is_shell_command) { + bool valid = false; + for (auto& sh : *s->shells) { + if (sh.name == u.shell_definition) { valid = true; break; } + } + set_valid(s->dropdown_shell_def, valid); + if (!valid && error_msg.empty()) + error_msg = "Shell definition not found: " + u.shell_definition; + } else { + set_valid(s->dropdown_shell_def, true); + } + + // Working directory: must exist as a directory if enabled + if (u.set_working_directory) { + bool valid = true; + if (u.working_directory.empty()) { + valid = false; + if (error_msg.empty()) error_msg = "Working directory is not defined"; + } else if (!fs::is_directory(u.working_directory)) { + valid = false; + if (error_msg.empty()) error_msg = "Working directory does not exist: " + u.working_directory; + } + set_valid(s->entry_workdir, valid); + } else { + set_valid(s->entry_workdir, true); + } + + // Rectifier: must exist and be executable if rectification enabled + if (u.rectify) { + bool valid = true; + if (u.rectifier.empty()) { + valid = false; + if (error_msg.empty()) error_msg = "Rectifier is not defined"; + } else if (!fs::exists(u.rectifier)) { + valid = false; + if (error_msg.empty()) error_msg = "Rectifier does not exist: " + u.rectifier; + } else if (access(u.rectifier.c_str(), X_OK) != 0) { + valid = false; + if (error_msg.empty()) error_msg = "Rectifier is not executable: " + u.rectifier; + } + set_valid(s->entry_rectifier, valid); + } else { + set_valid(s->entry_rectifier, true); + } + + // Environment file: must exist if supply_environment active + if (u.supply_environment) { + bool valid = true; + if (u.environment.empty()) { + valid = false; + if (error_msg.empty()) error_msg = "Environment file is not defined"; + } else if (!fs::exists(u.environment)) { + valid = false; + if (error_msg.empty()) error_msg = "Environment file does not exist: " + u.environment; + } + set_valid(s->entry_environment, valid); + } else { + set_valid(s->entry_environment, true); + } + + if (!error_msg.empty()) + s->project->report_status(error_msg); +} + static void populate_and_connect(DialogState* s) { auto& u = s->working_copy; @@ -390,6 +490,7 @@ static void populate_and_connect(DialogState* s) { gtk_editable_set_text(GTK_EDITABLE(s->entry_environment), u.environment.c_str()); update_sensitivity(s); s->loading = false; + validate_fields(s); // Entry bindings auto bind_entry = [s](GtkWidget* entry, std::string* target) { @@ -398,6 +499,7 @@ static void populate_and_connect(DialogState* s) { g_signal_connect(entry, "changed", G_CALLBACK(+[](GtkEditable* e, gpointer d) { auto* eb = static_cast(d); *eb->target = gtk_editable_get_text(e); + validate_fields(eb->state); }), eb); }; bind_entry(s->entry_name, &u.name); @@ -430,6 +532,7 @@ static void populate_and_connect(DialogState* s) { auto idx = gtk_drop_down_get_selected(GTK_DROP_DOWN(obj)); if (idx < s->shells->size()) s->working_copy.shell_definition = (*s->shells)[idx].name; + validate_fields(s); }), s); } diff --git a/src/views/main_window.cpp b/src/views/main_window.cpp index f003c1d..db2a4b3 100644 --- a/src/views/main_window.cpp +++ b/src/views/main_window.cpp @@ -225,16 +225,7 @@ void MainWindow::refresh_visible_page() { // Refresh the newly visible page if (visible == plan_view_->widget()) { - project_.load_all_units(); - if (!project_.plans.empty()) { - auto& plan = project_.plans[0]; - try { - plan = Plan::load(plan.filepath); - } catch (const std::exception& e) { - project_.report_status(std::string("Error reloading plan: ") + e.what()); - } - } - plan_view_->refresh(); + plan_view_->reload_plan_from_disk(); } else if (visible == units_view_->widget()) { units_view_->refresh(); } else if (visible == shells_view_->widget()) { diff --git a/src/views/plan_view.cpp b/src/views/plan_view.cpp index aed46eb..b0726d3 100644 --- a/src/views/plan_view.cpp +++ b/src/views/plan_view.cpp @@ -135,11 +135,13 @@ PlanView::PlanView(Project& project, GrexConfig& grex_config) gtk_frame_set_child(GTK_FRAME(task_ctrl_frame_), task_ctrl_box); gtk_box_append(GTK_BOX(unit_editor_->content_box()), task_ctrl_frame_); - // Name change callback to refresh the task list row label - unit_editor_->set_name_changed_callback([](const std::string&, void* data) { + // Callback fired after any unit edit in the dialog + unit_editor_->set_unit_edited_callback([](const std::string&, bool name_changed, void* data) { auto* self = static_cast(data); - self->plan_dirty_ = true; - gtk_widget_add_css_class(self->btn_save_plan_, "suggested-action"); + if (name_changed) { + self->plan_dirty_ = true; + gtk_widget_add_css_class(self->btn_save_plan_, "suggested-action"); + } if (self->current_task_idx_ >= 0) self->refresh_task_row(self->current_task_idx_); }, this); @@ -167,7 +169,7 @@ PlanView::PlanView(Project& project, GrexConfig& grex_config) g_signal_connect(btn_refresh, "clicked", G_CALLBACK(+[](GtkButton*, gpointer d) { auto* self = static_cast(d); - self->refresh(); + self->reload_plan_from_disk(); }), this); g_signal_connect(btn_delete_plan, "clicked", G_CALLBACK(on_delete_plan), this); @@ -195,8 +197,17 @@ void PlanView::populate_task_list() { for (auto& task : plan->tasks) { auto* row = gtk_list_box_row_new(); - auto text = std::string("\u25B6 ") + task.name; - auto* label = gtk_label_new(text.c_str()); + auto* label = gtk_label_new(nullptr); + auto* escaped = g_markup_escape_text(task.name.c_str(), -1); + Unit* unit = project_.find_unit(task.name); + bool valid = unit && project_.check_unit_valid(*unit); + std::string markup; + if (valid) + markup = std::string("\u25B6 ") + escaped; + else + markup = std::string("\u25B6 ") + escaped + ""; + g_free(escaped); + gtk_label_set_markup(GTK_LABEL(label), markup.c_str()); gtk_label_set_xalign(GTK_LABEL(label), 0.0f); gtk_widget_set_margin_start(label, 8); gtk_widget_set_margin_end(label, 8); @@ -217,8 +228,17 @@ void PlanView::refresh_task_row(int idx) { auto* label = gtk_list_box_row_get_child(GTK_LIST_BOX_ROW(row)); if (GTK_IS_LABEL(label)) { - auto text = std::string("\u25B6 ") + plan->tasks[idx].name; - gtk_label_set_text(GTK_LABEL(label), text.c_str()); + auto& task = plan->tasks[idx]; + auto* escaped = g_markup_escape_text(task.name.c_str(), -1); + Unit* unit = project_.find_unit(task.name); + bool valid = unit && project_.check_unit_valid(*unit); + std::string markup; + if (valid) + markup = std::string("\u25B6 ") + escaped; + else + markup = std::string("\u25B6 ") + escaped + ""; + g_free(escaped); + gtk_label_set_markup(GTK_LABEL(label), markup.c_str()); } } @@ -237,6 +257,10 @@ void PlanView::select_task(int idx) { project_.load_all_units(); Unit* unit = project_.find_unit(task.name); + if (unit) + project_.check_unit_valid(*unit); + else + project_.report_status("Unit not found: " + task.name); unit_editor_->load(&task, unit); update_move_buttons(); @@ -552,23 +576,27 @@ void PlanView::update_move_buttons() { gtk_widget_set_sensitive(btn_move_down_, current_task_idx_ < count - 1); } +void PlanView::reload_plan_from_disk() { + auto* plan = current_plan(); + if (!plan) return; + try { + *plan = Plan::load(plan->filepath); + project_.report_status("Reloaded plan: " + plan->filepath.filename().string()); + } catch (const std::exception& e) { + project_.report_status("Error reloading plan: " + std::string(e.what())); + } + refresh(); +} + void PlanView::refresh() { // reload units if paths now resolve project_.load_all_units(); - // reload the plan from disk if one is loaded auto* plan = current_plan(); - if (plan) { - try { - *plan = Plan::load(plan->filepath); - project_.report_status("Reloaded plan: " + plan->filepath.filename().string()); - } catch (const std::exception& e) { - project_.report_status("Error reloading plan: " + std::string(e.what())); - } + if (plan) gtk_label_set_markup(GTK_LABEL(plan_label_), (std::string("Plan: ") + plan->filepath.filename().string()).c_str()); - } else { + else gtk_label_set_markup(GTK_LABEL(plan_label_), "Plan: No plan loaded"); - } populate_task_list(); update_plan_buttons(); @@ -577,7 +605,7 @@ void PlanView::refresh() { } bool PlanView::is_dirty() const { - return plan_dirty_ || unit_editor_->is_dirty(); + return plan_dirty_; } void PlanView::save_dirty() { diff --git a/src/views/plan_view.h b/src/views/plan_view.h index 30d469a..b45fd6f 100644 --- a/src/views/plan_view.h +++ b/src/views/plan_view.h @@ -30,6 +30,7 @@ public: PlanView(Project& project, GrexConfig& grex_config); GtkWidget* widget() { return root_; } void refresh(); + void reload_plan_from_disk(); bool is_dirty() const; void save_dirty(); void revert_dirty(); diff --git a/src/views/shells_view.cpp b/src/views/shells_view.cpp index e25b066..0f641fb 100644 --- a/src/views/shells_view.cpp +++ b/src/views/shells_view.cpp @@ -417,7 +417,8 @@ void ShellsView::on_delete_file(GtkButton*, gpointer data) { auto* self = static_cast(data); if (!self->selected_file_) return; - auto name = self->selected_file_->filepath.filename().string(); + auto filepath = self->selected_file_->filepath; + auto name = filepath.filename().string(); auto it = std::find_if(self->project_.shell_files.begin(), self->project_.shell_files.end(), [&](const ShellsFile& sf) { return &sf == self->selected_file_; }); @@ -426,7 +427,13 @@ void ShellsView::on_delete_file(GtkButton*, gpointer data) { self->selected_file_ = nullptr; self->populate_file_list(); - self->project_.report_status("Removed shell file from project: " + name + " (file not deleted from disk)"); + + std::error_code ec; + if (std::filesystem::remove(filepath, ec)) + self->project_.report_status("Deleted shell file: " + name); + else + self->project_.report_status("Error: could not delete " + name + + (ec ? ": " + ec.message() : "")); } void ShellsView::on_new_shell(GtkButton*, gpointer data) { diff --git a/src/views/unit_editor.cpp b/src/views/unit_editor.cpp index 2f3f2d2..c8ac4aa 100644 --- a/src/views/unit_editor.cpp +++ b/src/views/unit_editor.cpp @@ -191,9 +191,9 @@ void UnitEditor::revert_current() { clear_dirty(); } -void UnitEditor::set_name_changed_callback(NameChangedCallback cb, void* data) { - name_cb_ = cb; - name_cb_data_ = data; +void UnitEditor::set_unit_edited_callback(UnitEditedCallback cb, void* data) { + edit_cb_ = cb; + edit_cb_data_ = data; } void UnitEditor::on_edit_unit(GtkButton*, gpointer data) { @@ -205,15 +205,17 @@ void UnitEditor::on_edit_unit(GtkButton*, gpointer data) { self->project_, self->grex_config_, self->project_.all_shells()); if (result == UnitDialogResult::Save) { - // Sync state in case the unit name changed in the dialog auto new_name = self->current_unit_->name; + bool name_changed = (new_name != self->current_unit_name_); self->current_unit_name_ = new_name; - if (self->current_task_) - self->current_task_->name = new_name; - gtk_label_set_text(GTK_LABEL(self->name_display_), new_name.c_str()); - if (self->name_cb_) - self->name_cb_(new_name, self->name_cb_data_); + if (name_changed) { + if (self->current_task_) + self->current_task_->name = new_name; + gtk_label_set_text(GTK_LABEL(self->name_display_), new_name.c_str()); + } self->mark_dirty(); + if (self->edit_cb_) + self->edit_cb_(new_name, name_changed, self->edit_cb_data_); } } @@ -234,7 +236,7 @@ void UnitEditor::on_select_unit(GtkButton*, gpointer data) { if (self->current_task_) self->current_task_->name = unit_name; Unit* unit = self->project_.find_unit(unit_name); self->load(self->current_task_, unit); - if (self->name_cb_) self->name_cb_(unit_name, self->name_cb_data_); + if (self->edit_cb_) self->edit_cb_(unit_name, true, self->edit_cb_data_); }); } diff --git a/src/views/unit_editor.h b/src/views/unit_editor.h index 08b04ea..91951d4 100644 --- a/src/views/unit_editor.h +++ b/src/views/unit_editor.h @@ -41,8 +41,8 @@ public: void save_current(); void revert_current(); - using NameChangedCallback = void(*)(const std::string& new_name, void* data); - void set_name_changed_callback(NameChangedCallback cb, void* data); + using UnitEditedCallback = void(*)(const std::string& new_name, bool name_changed, void* data); + void set_unit_edited_callback(UnitEditedCallback cb, void* data); private: Project& project_; @@ -62,8 +62,8 @@ private: GtkWidget* btn_save_unit_; GtkWidget* entry_comment_; - NameChangedCallback name_cb_ = nullptr; - void* name_cb_data_ = nullptr; + UnitEditedCallback edit_cb_ = nullptr; + void* edit_cb_data_ = nullptr; bool dirty_ = false; diff --git a/src/views/units_view.cpp b/src/views/units_view.cpp index f1cf828..e7d2fbc 100644 --- a/src/views/units_view.cpp +++ b/src/views/units_view.cpp @@ -145,8 +145,14 @@ UnitsView::UnitsView(Project& project, GrexConfig& grex_config) g_signal_connect(btn_move_up_, "clicked", G_CALLBACK(on_move_up), this); g_signal_connect(btn_move_down_, "clicked", G_CALLBACK(on_move_down), this); g_signal_connect(unit_listbox_, "row-activated", G_CALLBACK(on_unit_activated), this); - g_signal_connect(unit_listbox_, "row-selected", G_CALLBACK(+[](GtkListBox*, GtkListBoxRow*, gpointer d) { - static_cast(d)->update_move_buttons(); + g_signal_connect(unit_listbox_, "row-selected", G_CALLBACK(+[](GtkListBox*, GtkListBoxRow* row, gpointer d) { + auto* self = static_cast(d); + self->update_move_buttons(); + if (row && self->selected_file_) { + int idx = gtk_list_box_row_get_index(row); + if (idx >= 0 && idx < (int)self->selected_file_->units.size()) + self->project_.check_unit_valid(self->selected_file_->units[idx]); + } }), this); g_signal_connect(btn_refresh, "clicked", G_CALLBACK(+[](GtkButton*, gpointer d) { @@ -231,8 +237,15 @@ void UnitsView::populate_unit_list() { for (auto& u : selected_file_->units) { auto* row = gtk_list_box_row_new(); - auto text = std::string("\u2022 ") + u.name; - auto* label = gtk_label_new(text.c_str()); + auto* label = gtk_label_new(nullptr); + auto* escaped = g_markup_escape_text(u.name.c_str(), -1); + std::string markup; + if (project_.check_unit_valid(u)) + markup = std::string("\u2022 ") + escaped; + else + markup = std::string("\u2022 ") + escaped + ""; + g_free(escaped); + gtk_label_set_markup(GTK_LABEL(label), markup.c_str()); gtk_label_set_xalign(GTK_LABEL(label), 0.0f); gtk_widget_set_margin_start(label, 8); gtk_widget_set_margin_end(label, 8); @@ -403,7 +416,8 @@ void UnitsView::on_delete_file(GtkButton*, gpointer data) { auto* self = static_cast(data); if (!self->selected_file_) return; - auto name = self->selected_file_->filepath.filename().string(); + auto filepath = self->selected_file_->filepath; + auto name = filepath.filename().string(); auto it = std::find_if(self->project_.unit_files.begin(), self->project_.unit_files.end(), [&](const UnitFile& uf) { return &uf == self->selected_file_; }); @@ -412,7 +426,13 @@ void UnitsView::on_delete_file(GtkButton*, gpointer data) { self->selected_file_ = nullptr; self->populate_file_list(); - self->project_.report_status("Removed unit file from project: " + name + " (file not deleted from disk)"); + + std::error_code ec; + if (std::filesystem::remove(filepath, ec)) + self->project_.report_status("Deleted unit file: " + name); + else + self->project_.report_status("Error: could not delete " + name + + (ec ? ": " + ec.message() : "")); } void UnitsView::on_new_unit(GtkButton*, gpointer data) {