Skip to content

Commit e1ed3ec

Browse files
suveshmozasuveshmoza
committed
fix(excmds): clear excmd misuse notification after successful execution
1 parent 7df43a9 commit e1ed3ec

4 files changed

Lines changed: 256 additions & 2 deletions

File tree

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ dist/
77
compile_commands.json
88
.envrc.local
99
/src/glide.ts
10+
.vscode/

‎src/glide/browser/base/content/browser-commandline.mts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,21 @@ export class TabsCompletionSource implements GlideCompletionSource<TabCompletion
214214
],
215215
}),
216216

217-
async accept() {
218-
gBrowser.selectedTab = tab;
217+
async accept(ctx) {
218+
const input = ctx.input.trim();
219+
const parts = input.split(" ");
220+
const arg = parts[1];
221+
222+
// User typed "tab " with no arg - select the focused tab from completion list
223+
if (arg === undefined || arg === "") {
224+
gBrowser.selectedTab = tab;
225+
GlideBrowser.remove_notification("glide-excmd-error");
226+
return;
227+
}
228+
229+
// User typed "tab <something>" - execute through GlideExcmds
230+
// This handles both valid indices and invalid input (which will show an error)
231+
await GlideExcmds.execute(input as any);
219232
},
220233
async delete() {
221234
gBrowser.removeTab(this.tab);

‎src/glide/browser/base/content/browser-excmds.mts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,17 @@ class GlideExcmdsClass {
106106
command: glide.ExcmdValue,
107107
props?: SetOptional<ExecuteProps, "args">,
108108
): Promise<void> {
109+
const command_name = typeof command === "string" ? extract_command_name(command) : command.name;
110+
109111
try {
110112
await this.#execute(command, {
111113
...props,
112114
args: { ...this.#parse_args(command), tab_id: GlideBrowser.active_tab_id, ...props?.args },
113115
});
116+
117+
if (!command_name.includes("commandline_")) {
118+
GlideBrowser.remove_notification("glide-excmd-error");
119+
}
114120
} catch (err) {
115121
GlideBrowser._log.error(err);
116122

‎src/glide/browser/base/content/test/commandline/browser_commandline.ts‎

Lines changed: 234 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -725,3 +725,237 @@ add_task(async function test_commandline_close() {
725725
is(result, true, "close() should return true when the commandline was open");
726726
await wait_for_mode("normal");
727727
});
728+
729+
add_task(async function test_error_notification_tab_missing_args() {
730+
await reload_config(function _() {});
731+
732+
await BrowserTestUtils.withNewTab(FILE, async () => {
733+
await keys(":tab<CR>");
734+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
735+
736+
const notification = gNotificationBox.getNotificationWithValue("glide-excmd-error")!;
737+
ok(
738+
notification.shadowRoot.querySelector(".message")?.textContent?.includes("tab_index"),
739+
"error message should mention missing 'tab_index' argument",
740+
);
741+
gNotificationBox.removeNotification(notification);
742+
});
743+
});
744+
745+
add_task(async function test_error_notification_tab_invalid_index() {
746+
await reload_config(function _() {});
747+
748+
await BrowserTestUtils.withNewTab(FILE, async () => {
749+
await keys(":tab 999<CR>");
750+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
751+
752+
const notification = gNotificationBox.getNotificationWithValue("glide-excmd-error")!;
753+
ok(
754+
notification.shadowRoot.querySelector(".message")?.textContent?.includes("could not find a tab"),
755+
"error message should indicate tab not found",
756+
);
757+
gNotificationBox.removeNotification(notification);
758+
});
759+
});
760+
761+
add_task(async function test_error_notification_set_missing_args() {
762+
await reload_config(function _() {});
763+
764+
await BrowserTestUtils.withNewTab(FILE, async () => {
765+
await keys(":set<CR>");
766+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
767+
768+
gNotificationBox.removeNotification(gNotificationBox.getNotificationWithValue("glide-excmd-error")!);
769+
});
770+
});
771+
772+
add_task(async function test_error_notification_set_invalid_option() {
773+
await reload_config(function _() {});
774+
775+
await BrowserTestUtils.withNewTab(FILE, async () => {
776+
await keys(":set invalid_option_name 123<CR>");
777+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
778+
779+
const notification = gNotificationBox.getNotificationWithValue("glide-excmd-error")!;
780+
ok(
781+
notification.shadowRoot.querySelector(".message")?.textContent?.includes("not a valid option"),
782+
"error message should indicate invalid option",
783+
);
784+
gNotificationBox.removeNotification(notification);
785+
});
786+
});
787+
788+
add_task(async function test_error_notification_mode_change_missing_args() {
789+
await reload_config(function _() {});
790+
791+
await BrowserTestUtils.withNewTab(FILE, async () => {
792+
await keys(":mode_change<CR>");
793+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
794+
795+
gNotificationBox.removeNotification(gNotificationBox.getNotificationWithValue("glide-excmd-error")!);
796+
});
797+
});
798+
799+
add_task(async function test_error_notification_keys_missing_args() {
800+
await reload_config(function _() {});
801+
802+
await BrowserTestUtils.withNewTab(FILE, async () => {
803+
await keys(":keys<CR>");
804+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
805+
806+
const notification = gNotificationBox.getNotificationWithValue("glide-excmd-error")!;
807+
ok(
808+
notification.shadowRoot.querySelector(".message")?.textContent?.includes("keyseq"),
809+
"error message should mention missing 'keyseq' argument",
810+
);
811+
gNotificationBox.removeNotification(notification);
812+
});
813+
});
814+
815+
add_task(async function test_error_notification_unmap_missing_args() {
816+
await reload_config(function _() {});
817+
818+
await BrowserTestUtils.withNewTab(FILE, async () => {
819+
await keys(":unmap<CR>");
820+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
821+
822+
const notification = gNotificationBox.getNotificationWithValue("glide-excmd-error")!;
823+
ok(
824+
notification.shadowRoot.querySelector(".message")?.textContent?.includes("lhs"),
825+
"error message should mention missing 'lhs' argument",
826+
);
827+
gNotificationBox.removeNotification(notification);
828+
});
829+
});
830+
831+
add_task(async function test_error_notification_caret_move_missing_args() {
832+
await reload_config(function _() {});
833+
834+
await BrowserTestUtils.withNewTab(FILE, async () => {
835+
await keys(":caret_move<CR>");
836+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
837+
838+
const notification = gNotificationBox.getNotificationWithValue("glide-excmd-error")!;
839+
ok(
840+
notification.shadowRoot.querySelector(".message")?.textContent?.includes("direction"),
841+
"error message should mention missing 'direction' argument",
842+
);
843+
gNotificationBox.removeNotification(notification);
844+
});
845+
});
846+
847+
add_task(async function test_error_notification_cleared_tab_succeeds() {
848+
await reload_config(function _() {});
849+
850+
await BrowserTestUtils.withNewTab(FILE, async () => {
851+
await keys(":tab<CR>");
852+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
853+
854+
await keys(":tab 0<CR>");
855+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error") === null).ok();
856+
857+
is(
858+
gNotificationBox.getNotificationWithValue("glide-excmd-error"),
859+
null,
860+
"error notification should be cleared after successful tab command",
861+
);
862+
});
863+
});
864+
865+
add_task(async function test_error_notification_cleared_mode_change_succeeds() {
866+
await reload_config(function _() {});
867+
868+
await BrowserTestUtils.withNewTab(FILE, async () => {
869+
await keys(":mode_change<CR>");
870+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
871+
872+
await keys(":mode_change normal<CR>");
873+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error") === null).ok();
874+
875+
is(
876+
gNotificationBox.getNotificationWithValue("glide-excmd-error"),
877+
null,
878+
"error notification should be cleared after successful mode_change command",
879+
);
880+
});
881+
});
882+
883+
add_task(async function test_error_notification_cleared_by_different_command() {
884+
await reload_config(function _() {});
885+
886+
await BrowserTestUtils.withNewTab(FILE, async () => {
887+
await keys(":tab<CR>");
888+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
889+
890+
await keys(":reload<CR>");
891+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error") === null).ok();
892+
893+
is(
894+
gNotificationBox.getNotificationWithValue("glide-excmd-error"),
895+
null,
896+
"error notification should be cleared after a different successful command",
897+
);
898+
});
899+
});
900+
901+
add_task(async function test_error_notification_cleared_by_different_command_with_args() {
902+
await reload_config(function _() {});
903+
904+
await BrowserTestUtils.withNewTab(FILE, async () => {
905+
await keys(":set<CR>");
906+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
907+
908+
await keys(":mode_change normal<CR>");
909+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error") === null).ok();
910+
911+
is(
912+
gNotificationBox.getNotificationWithValue("glide-excmd-error"),
913+
null,
914+
"error notification should be cleared after a different successful command with args",
915+
);
916+
});
917+
});
918+
919+
add_task(async function test_error_notification_cleared_via_tab_completion() {
920+
await reload_config(function _() {});
921+
922+
await BrowserTestUtils.withNewTab(FILE, async () => {
923+
await keys(":tab<CR>");
924+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
925+
926+
await GlideTestUtils.commandline.open();
927+
await keys("tab ");
928+
await waiter(() => GlideTestUtils.commandline.current_source_header()).is("tabs");
929+
await waiter(() => GlideTestUtils.commandline.visible_rows().length > 0).ok();
930+
await keys("<CR>");
931+
932+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error") === null).ok();
933+
934+
is(
935+
gNotificationBox.getNotificationWithValue("glide-excmd-error"),
936+
null,
937+
"error notification should be cleared after selecting tab via completion",
938+
);
939+
});
940+
});
941+
942+
add_task(async function test_multiple_errors_cleared_by_one_success() {
943+
await reload_config(function _() {});
944+
945+
await BrowserTestUtils.withNewTab(FILE, async () => {
946+
await keys(":tab<CR>");
947+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
948+
949+
await keys(":set<CR>");
950+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error")).ok();
951+
952+
await keys(":tab 0<CR>");
953+
await waiter(() => gNotificationBox.getNotificationWithValue("glide-excmd-error") === null).ok();
954+
955+
is(
956+
gNotificationBox.getNotificationWithValue("glide-excmd-error"),
957+
null,
958+
"error notification should be cleared after any successful command",
959+
);
960+
});
961+
});

0 commit comments

Comments
 (0)