Skip to content

fix: 't' key tag editing on fresh entries and allow last-tag removal#115

Open
manato-tajiri wants to merge 1 commit into
Adembc:mainfrom
manato-tajiri:main
Open

fix: 't' key tag editing on fresh entries and allow last-tag removal#115
manato-tajiri wants to merge 1 commit into
Adembc:mainfrom
manato-tajiri:main

Conversation

@manato-tajiri
Copy link
Copy Markdown

Description

This PR fixes two bugs related to the t key tag editing feature:

Bug 1: t key silently fails on fresh entries (never edited via e)

Root cause: In handlers.go:411, the showEditTagsForm save callback discards the error from UpdateServer with _ = t.serverService.UpdateServer(...). If the SSH config write fails (e.g., permissions issue), the tags are never saved but the UI shows "Tags updated" as if it succeeded.

Fix: The error is now properly surfaced via an error modal, matching the behavior of handleServerSave (used by the e key).

Bug 2: Selection jumps to first entry after tag save

Root cause: In server_list.go:85, UpdateServers always resets the selected item to index 0 after refreshing the list, making it appear that tags were not saved on the edited server.

Fix: UpdateServers now tracks the currently selected server alias before refresh and restores the selection to that same server afterward.

Notes

  • The nil-guard fix in metadata_manager.go (for clearing the last tag) was already present in the codebase — no changes needed there.
  • All existing tests pass.

Closes #(issue if applicable)

@manato-tajiri
Copy link
Copy Markdown
Author

Hi @Adembc — this PR fixes two bugs in the tag editing flow (t key):

  1. handlers.go:411: The error from UpdateServer is silently discarded (_ = ...). If the SSH config write fails, tags are never saved but the UI says "Tags updated." Fixed by surfacing errors via a modal (matching handleServerSave behavior used by the e key).

  2. server_list.go:85: UpdateServers resets selection to index 0 after every refresh, making it appear tags weren't saved on the edited server. Fixed by tracking the current server alias and restoring selection after refresh.

CC: I've also filed issue #116 documenting this bug from a user report. Would appreciate a review when you have a moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant