Skip to content

DO NOT MERGE 2 #609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
karlclement wants to merge 7 commits into
base: derrick/cod-1379-if-there-is-no-remote-on-a-local
Choose a base branch
from
Open

DO NOT MERGE 2 #609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3ae3260
COD-1266 Correct notification count in VS Code bar (#463)
meichend Jul 22, 2022
14ead08
let the user know that we couldn't find any remotes (#462)
derrickpersson Jul 22, 2022
d43d129
Version bump and changelog
derrickpersson Jul 22, 2022
a957c1a
Alerting only when we check for repo's (#465)
derrickpersson Jul 22, 2022
95b595f
Merge pull request #464 from usecodex/releases/v0.5.5
derrickpersson Jul 22, 2022
92ef349
Merge pull request #466 from usecodex/production
derrickpersson Jul 22, 2022
009d7b3
Fixing bug where notifications would disappear
derrickpersson Jul 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions codex-vscode/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,18 @@
All notable changes to the "codex" extension will be documented in this file.


## [0.5.5] - 2022-07-22

### Added
- N/A

### Changed
- N/A

### Fixed
- Show the correct unread notification count in the VS Code status bar
- Surface the error for repositories which do not have remotes

## [0.5.4] - 2022-07-19

### Added
Expand Down
4 changes: 2 additions & 2 deletions codex-vscode/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion codex-vscode/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"name": "codex-build",
"displayName": "Codex",
"description": "Codex makes programming multiplayer.",
"version": "0.5.4",
"version": "0.5.5",
"icon": "media/codex.png",
"author": "Codex Build Inc.",
"repository": {},
Expand Down
25 changes: 25 additions & 0 deletions codex-vscode/src/commands/extension/statusBarUpdate.command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import type { Notification } from "@usecodex/common-library";
import { MESSAGE_TYPES } from "../../globals";
import { handleSetStatusBarMessage, StatusBarData } from "../../helpers/notifications.helper";
import BaseCommand from "../base/base.command";
import { ExecutedCommandResponse } from "../base/command";

export default class StatusBarUpdateCommand extends BaseCommand<Notification> {
public messageType: MESSAGE_TYPES = MESSAGE_TYPES.statusBarUpdate;
public statusData: StatusBarData;

constructor(statusData: StatusBarData) {
super();
this.statusData = statusData;
}

async execute(): Promise<ExecutedCommandResponse<MESSAGE_TYPES, Notification>> {
try {
handleSetStatusBarMessage(this.statusData);

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider validating statusData before passing it to handleSetStatusBarMessage to ensure it meets expected structure and content, enhancing security and robustness.

return this.formatSuccessResponse();
} catch (err) {
this.logError(err);
return this.formatErrorResponse();
}
}
}
2 changes: 0 additions & 2 deletions codex-vscode/src/commands/notification/deleteNotification.command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { Notification } from "@usecodex/common-library";
import { MESSAGE_TYPES } from "../../globals";
import { handleStatusBarUpdate } from "../../helpers/notifications.helper";
import { UserService } from "../../services/user.service";

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of handleStatusBarUpdate import suggests that the function is no longer used; ensure that its removal does not affect other parts of the system that might rely on it. If it's indeed unused, the deletion is appropriate.

import BaseCommand from "../base/base.command";
import { ExecutedCommandResponse } from "../base/command";
Expand All @@ -18,7 +17,6 @@ export default class DeleteNotificationCommand extends BaseCommand<Notification>
async execute(): Promise<ExecutedCommandResponse<MESSAGE_TYPES, Notification>> {
try {
await UserService.getInstance().getNotificationService().delete(this.notification.id);
await handleStatusBarUpdate();

this.logAnalytics(new ContentEventAnalyticsData(this.notification));

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of the handleStatusBarUpdate() call may lead to the status bar not being updated to reflect the deletion of a notification. If this update is necessary for the UI to remain consistent, consider re-adding this call or ensuring that the status bar is updated elsewhere in the code.

return this.formatSuccessResponse(this.notification);
Expand Down
2 changes: 0 additions & 2 deletions codex-vscode/src/commands/notification/deleteNotifications.command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { Notification } from "@usecodex/common-library";
import { MESSAGE_TYPES } from "../../globals";
import { handleStatusBarUpdate } from "../../helpers/notifications.helper";
import { UserService } from "../../services/user.service";

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of handleStatusBarUpdate import suggests that the status bar update functionality is no longer needed. Confirm that this functionality is indeed obsolete or ensure that its removal does not affect other parts of the application that might rely on it.

import BaseCommand from "../base/base.command";
import { ExecutedCommandResponse } from "../base/command";
Expand All @@ -18,7 +17,6 @@ export default class DeleteNotificationsCommand extends BaseCommand<Notification
async execute(): Promise<ExecutedCommandResponse<MESSAGE_TYPES, Notification[]>> {
try {
await UserService.getInstance().getNotificationService().deleteMany(this.notifications);
await handleStatusBarUpdate();

this.logAnalytics(new NonSensitiveEventAnalyticsData({}));

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of handleStatusBarUpdate() may affect the UI's responsiveness to the delete operation. If this update is intentional and the status bar should no longer reflect changes, no action is needed; otherwise, consider re-adding this call to ensure the UI remains in sync with the operation's state.

return this.formatSuccessResponse(this.notifications);
Expand Down
2 changes: 0 additions & 2 deletions codex-vscode/src/commands/notification/markAllNotificationRead.command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Notification } from "@usecodex/common-library";
import { MESSAGE_TYPES } from "../../globals";
import { handleSetStatusBarMessage } from "../../helpers/notifications.helper";
import { UserService } from "../../services/user.service";

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of 'handleSetStatusBarMessage' import suggests it is no longer used. Confirm its removal is intentional and that there are no unused references to it in the codebase.

import BaseCommand from "../base/base.command";
import { ExecutedCommandResponse } from "../base/command";
Expand All @@ -24,7 +23,6 @@ export default class MarkAllNotificationsReadCommand extends BaseCommand<Notific
await UserService.getInstance()
.getNotificationService()
.updateMany(readNotifications as Array<Notification>);
handleSetStatusBarMessage(readNotifications);

this.logAnalytics(new NonSensitiveEventAnalyticsData({}));

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of handleSetStatusBarMessage(readNotifications); may affect the user interface by not updating the status bar to reflect the change in notification status. If this is intentional and the status bar update is handled elsewhere, ensure that the functionality is covered by tests to prevent regression.

return this.formatSuccessResponse(readNotifications);
Expand Down
2 changes: 0 additions & 2 deletions codex-vscode/src/commands/notification/updateNotification.command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Notification } from "@usecodex/common-library";
import { MESSAGE_TYPES } from "../../globals";
import { handleStatusBarUpdate } from "../../helpers/notifications.helper";
import { UserService } from "../../services/user.service";

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of handleStatusBarUpdate import suggests it is no longer used. Verify if this is intentional and ensure that any related functionality is either removed or refactored accordingly.

import BaseCommand from "../base/base.command";
import { ExecutedCommandResponse } from "../base/command";
Expand All @@ -19,7 +18,6 @@ export default class UpdateNotificationCommand extends BaseCommand<Notification>
await UserService.getInstance()
.getNotificationService()
.update(this.notification as Notification);
await handleStatusBarUpdate();
return this.formatSuccessResponse(this.notification);
} catch (err) {

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of handleStatusBarUpdate() may lead to the status bar not being updated after a notification is changed. If this update is necessary for the UI to reflect changes, re-add the handleStatusBarUpdate() call after the notification update succeeds.

this.logError(err);
Expand Down
18 changes: 15 additions & 3 deletions codex-vscode/src/commands/repositories/getRepositories.command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,27 @@
import * as vscode from "vscode";
import { MESSAGE_TYPES } from "../../globals";
import { VSCodeRepository } from "../../models/shared/vscodeRepository.model";

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the removed WorkspaceService is no longer needed by the GetRepositoriesCommand. If it is still required, it should be re-imported and used appropriately.

import { WorkspaceService } from "../../services/workspace.service";
import { GitService } from "../../services/git.service";
import BaseCommand from "../base/base.command";

export default class GetRepositoriesCommand extends BaseCommand<VSCodeRepository[]> {
public messageType: MESSAGE_TYPES = MESSAGE_TYPES.getRepositories;

async execute() {
try {
const workspaceService = await WorkspaceService.getInstance();
const repositories: VSCodeRepository[] = workspaceService.repositories;
const gitService = await GitService.getInstance();

const remoteCheck = gitService.checkForRemotes();

remoteCheck
.filter((it) => !it.hasRemote)
.forEach((repoCheck) => {
vscode.window.showErrorMessage(
`Uh-oh. No remote detected for this repo: ${repoCheck.rootUri}. Please connect a remote to continue.`
);
});
Comment on lines +16 to +22

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider handling the case where multiple repositories lack remotes more gracefully. Accumulating error messages and displaying them in a single dialog or notification could improve user experience by reducing potential spamming of error messages.


const repositories = await gitService.getRepositories();
return this.formatSuccessResponse(repositories);
} catch (err) {
this.logError(err);
Expand Down
2 changes: 2 additions & 0 deletions codex-vscode/src/globals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ const enum MESSAGE_TYPES {
// URI
generateFileUri = "generateFileUri",
generateContextUri = "generateContextUri",

statusBarUpdate = "statusBarUpdate",

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ensure the new enum member 'statusBarUpdate' follows the naming convention of the existing members for consistency.

}

const enum NAVIGATION_ERROR_TYPES {
Expand Down
4 changes: 2 additions & 2 deletions codex-vscode/src/helpers/handlers/pusherEvent.handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
UserNotificationType,
} from "@usecodex/common-library";
import { MESSAGE_TYPES, PUSHER_EVENT_TYPES } from "../../globals";
import { displayInformationMessage, handleStatusBarUpdate } from "../notifications.helper";
import { displayInformationMessage } from "../notifications.helper";

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of handleStatusBarUpdate from the imports suggests it is no longer used in this file. Verify if this function is indeed unused and remove any dead code associated with it to maintain code cleanliness.


import logger from "../logging.helper";
import { WorkspaceService } from "../../services/workspace.service";
Expand Down Expand Up @@ -66,11 +66,11 @@ export async function pusherEventHandler(event: any, data: any) {
}
}
}
data.notification && handleStatusBarUpdate();

// Temporarly disable check for allowNotifications, in Terminal Velocity, we will re-implement this setting.
if (data.notification /* && LocalStorageManager.get(LOCAL_STORAGE_KEYS.allowNotifications) */) {
displayInformationMessage(data.notification);
SidebarService.getInstance().postMessage(MESSAGE_TYPES.statusBarUpdate);
}

data.notification && SidebarService.getInstance().postMessage(MESSAGE_TYPES.getNotifications, data.notification); // TODO: make this an array of 1 object
Expand Down
16 changes: 16 additions & 0 deletions codex-vscode/src/helpers/handlers/webviewOutgoingMessage.handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import GenerateContextUri from "../../commands/uri/contextUri.command";
import GetSearchSuggestionsCommand from "../../commands/search/getSearchSuggestions.command";
import ExtensionVersion from "../../commands/client/extensionVersion.command";
import ExtensionEnvironment from "../../commands/client/extensionEnvironment.command";
import StatusBarUpdateCommand from "../../commands/extension/statusBarUpdate.command";

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the newly imported StatusBarUpdateCommand is utilized within the code if it's necessary for the functionality, otherwise remove the import to avoid unused dependencies.


export async function sidebarMessageHandler(
message: WebviewOutgoingMessageTypes,
Expand Down Expand Up @@ -252,28 +253,33 @@ export async function sidebarMessageHandler(
case MESSAGE_TYPES.getNotifications: {
const getNotifications: GetNotificationsCommand = new GetNotificationsCommand();
clientView.webview.postMessage(await getNotifications.execute());
sendStatusBarUpdateMessage(clientView);
Comment on lines 255 to +256

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that sendStatusBarUpdateMessage is called only after verifying the success of the preceding execute call to prevent misleading status updates in case of command failure.

break;
}
case MESSAGE_TYPES.updateNotification: {
const markNotificationReadCommand: UpdateNotificationCommand = new UpdateNotificationCommand(message.value);
clientView.webview.postMessage(await markNotificationReadCommand.execute());
sendStatusBarUpdateMessage(clientView);
Comment on lines 261 to +262

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verify that sendStatusBarUpdateMessage is called only after confirming the successful execution of UpdateNotificationCommand to avoid incorrect status bar updates on command failure.

break;
}
case MESSAGE_TYPES.markAllNotificationRead: {
const markAllNotificationReadCommand: MarkAllNotificationsReadCommand = new MarkAllNotificationsReadCommand(
message.value
);
clientView.webview.postMessage(await markAllNotificationReadCommand.execute());
sendStatusBarUpdateMessage(clientView);
break;
}
case MESSAGE_TYPES.deleteNotification: {
const deleteNotificationCommand: DeleteNotificationCommand = new DeleteNotificationCommand(message.value);
clientView.webview.postMessage(await deleteNotificationCommand.execute());
sendStatusBarUpdateMessage(clientView);
break;
}
case MESSAGE_TYPES.deleteNotifications: {
const deleteNotificationsCommand: DeleteNotificationsCommand = new DeleteNotificationsCommand(message.value);
clientView.webview.postMessage(await deleteNotificationsCommand.execute());
sendStatusBarUpdateMessage(clientView);
Comment on lines +270 to +282

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the sendStatusBarUpdateMessage function is defined and properly handles any potential exceptions, as it is being called after postMessage operations which could throw errors.

break;
}
case MESSAGE_TYPES.gotoCodexNotificationObject: {
Expand Down Expand Up @@ -415,6 +421,11 @@ export async function sidebarMessageHandler(
clientView.webview.postMessage(await new ExtensionEnvironment().execute());
break;
}
case MESSAGE_TYPES.statusBarUpdate: {
const statusBarUpdate = new StatusBarUpdateCommand(message.value);

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the 'message.value' passed to the StatusBarUpdateCommand constructor is validated or sanitized to prevent potential security risks such as code injection.

await statusBarUpdate.execute();
break;
}
case MESSAGE_TYPES.trackEvent: {
vscode.commands.executeCommand(COMMAND_TYPES.trackEvent, message.value);
break;
Expand All @@ -425,3 +436,8 @@ export async function sidebarMessageHandler(
}
}
}
function sendStatusBarUpdateMessage(clientView: vscode.WebviewView) {
clientView.webview.postMessage({
type: MESSAGE_TYPES.statusBarUpdate,
});
Comment on lines +441 to +442

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a payload to the statusBarUpdate message to ensure the client can handle the update appropriately.

}
12 changes: 5 additions & 7 deletions codex-vscode/src/helpers/notifications.helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@ import { MESSAGE_TYPES } from "../globals";
const OPEN = "Open";
const OK = "Ok";

async function handleStatusBarUpdate(): Promise<vscode.Disposable> {
const notifications = await UserService.getInstance().getNotifications();
const notificationMessage = formatStatusBarMessage(notifications);
return updateStatusBar(notificationMessage, vscode.window);
export interface StatusBarData {
notifications: Notification[];
}

function handleSetStatusBarMessage(notifications: Array<Notification>): vscode.Disposable {
const notificationMessage = formatStatusBarMessage(notifications);
function handleSetStatusBarMessage(statusData: StatusBarData): vscode.Disposable {
const notificationMessage = formatStatusBarMessage(statusData.notifications);
return updateStatusBar(notificationMessage, vscode.window);

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactoring from a direct array of notifications to using a StatusBarData interface is a good encapsulation. However, ensure that all callers of handleSetStatusBarMessage are updated to pass the new StatusBarData structure instead of the raw array.

}

Expand Down Expand Up @@ -54,4 +52,4 @@ function displayInformationMessage(notification: Notification) {
});
}

export { handleStatusBarUpdate, handleSetStatusBarMessage, formatStatusBarMessage, displayInformationMessage };
export { handleSetStatusBarMessage, formatStatusBarMessage, displayInformationMessage };

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of handleStatusBarUpdate from the export statement suggests that it is no longer used. Confirm that this function has been fully deprecated or replaced, and if so, ensure that all references to it have been removed from the codebase to prevent dead code.

9 changes: 8 additions & 1 deletion codex-vscode/src/models/webview/webviewOutgoingMessage.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ export type WebviewOutgoingMessageTypes =
| GetExtensionVersion
| GetExtensionEnvironment
| TrackPageMessage
| TrackEventMessage;
| TrackEventMessage
| StatusBarUpdate;

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the new StatusBarUpdate message type is handled appropriately in the corresponding message handling logic to maintain the integrity of the system's messaging functionality.


export interface VSCodeMessage<T, V> {
type: T;
Expand Down Expand Up @@ -201,6 +202,8 @@ type GetExtensionEnvironment = VSCodeMessage<MESSAGE_TYPES.getExtensionEnvironme
type TrackEventMessage = VSCodeMessage<MESSAGE_TYPES.trackEvent, TrackEventMessageValue>;
type TrackPageMessage = VSCodeMessage<MESSAGE_TYPES.trackPage, TrackPageMessageValue>;

type StatusBarUpdate = VSCodeMessage<MESSAGE_TYPES.statusBarUpdate, StatusBarData>;

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the new 'StatusBarUpdate' type includes all necessary properties for updating the status bar, and consider defining a schema or interface for 'StatusBarData' to enforce type safety and data structure consistency.


interface CodexBaseMessageValue {
codexId: number;
}
Expand Down Expand Up @@ -254,3 +257,7 @@ export interface TrackPageMessageValue extends CodexBaseMessageValue {
category?: string;
properties?: { [key: string]: any };

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'any' type for the 'properties' value in the interface may lead to runtime errors and lacks type safety. Consider defining a more specific type or using generics to enforce type constraints.

}

export interface StatusBarData {
notifications: Notification[];
}
9 changes: 9 additions & 0 deletions codex-vscode/src/services/git.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,15 @@ export class GitService implements Singleton {
}
}

public checkForRemotes(): { rootUri: string; hasRemote: boolean }[] {
return this.gitAPI.repositories.map((repository) => {
return {
rootUri: repository.rootUri.toString(),
hasRemote: repository?.state?.remotes?.length > 0,
Comment on lines +126 to +129

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method checkForRemotes assumes this.gitAPI and this.gitAPI.repositories are not null. To prevent potential runtime errors, add a null check for this.gitAPI and this.gitAPI.repositories before accessing the repositories property.

};
});
}

/**
* Retrieves a remote repository; ideally returns the Origin, but if not, returns a fallback
* @param repository
Expand Down
2 changes: 2 additions & 0 deletions codex-webview/src/globals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ const enum MESSAGE_TYPES {
// URI
generateFileUri = "generateFileUri",
generateContextUri = "generateContextUri",

statusBarUpdate = "statusBarUpdate",

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ensure the new enum member 'statusBarUpdate' follows the naming convention of the existing members for consistency.

}

const enum COMMAND_TYPES {
Expand Down
8 changes: 7 additions & 1 deletion codex-webview/src/models/shared/webviewIncomingMessage.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export type WebviewIncomingMessageTypes =
| SetExtensionVersion
| GetExtensionEnvironment
| SetExtensionEnvironment
| StatusBarUpdate

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addition of 'StatusBarUpdate' to the message types should be accompanied by corresponding handler implementation to ensure the new message type is processed correctly.

// Navigation
| NavigateVerificationPage
| NavigateOnboardingInviteTeamPage
Expand Down Expand Up @@ -382,7 +383,7 @@ type GetExtensionVersion = { type: MESSAGE_TYPES.getExtensionVersion; value: und
type SetExtensionVersion = ExecutedCommandResponse<MESSAGE_TYPES.setExtensionVersion, { version: string }>;
type GetExtensionEnvironment = { type: MESSAGE_TYPES.getExtensionEnvironment; value: undefined };
type SetExtensionEnvironment = ExecutedCommandResponse<MESSAGE_TYPES.setExtensionEnvironment, { environment: string }>;

type StatusBarUpdate = ExecutedCommandResponse<MESSAGE_TYPES.statusBarUpdate, StatusBarUpdateData>;

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider defining a type for the 'StatusBarUpdateData' used in the 'StatusBarUpdate' type to ensure type safety and maintainability.

// TODO: Break out navigation into own message type.
type NavigateNewContextItemPage = {
type: MESSAGE_TYPES.navigateNewContextItemPage;
Expand Down Expand Up @@ -433,3 +434,8 @@ type GoTo = {
value: Context[];
};
type NotFound = { type: MESSAGE_TYPES.notFound; value?: { message?: string } };

type StatusBarUpdateData = {
type: MESSAGE_TYPES.statusBarUpdate;
value: undefined;

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'value' field in 'StatusBarUpdateData' should not be explicitly set to 'undefined'. If the intention is to have an optional 'value', simply omit the field or use 'value?: any'.

};
9 changes: 8 additions & 1 deletion codex-webview/src/models/webview/webviewOutgoingMessage.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ export type WebviewOutgoingMessageTypes =
| GetExtensionVersion
| GetExtensionEnvironment
| TrackPageMessage
| TrackEventMessage;
| TrackEventMessage
| StatusBarUpdate;

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ensure that the new StatusBarUpdate message type is handled wherever WebviewOutgoingMessageTypes are processed to maintain functionality.


export interface VSCodeMessage<T, V> {
type: T;
Expand Down Expand Up @@ -201,6 +202,8 @@ type GetExtensionEnvironment = VSCodeMessage<MESSAGE_TYPES.getExtensionEnvironme
type TrackEventMessage = VSCodeMessage<MESSAGE_TYPES.trackEvent, TrackEventMessageValue>;
type TrackPageMessage = VSCodeMessage<MESSAGE_TYPES.trackPage, TrackPageMessageValue>;

type StatusBarUpdate = VSCodeMessage<MESSAGE_TYPES.statusBarUpdate, StatusBarData>;

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the new 'StatusBarUpdate' type includes all necessary properties for the status bar data and adheres to the established message format conventions.


interface CodexBaseMessageValue {
codexId: number;
}
Expand Down Expand Up @@ -254,3 +257,7 @@ export interface TrackPageMessageValue extends CodexBaseMessageValue {
category?: string;
properties?: { [key: string]: any };

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider defining a more specific type for the 'properties' object values instead of 'any' to ensure type safety and prevent potential runtime errors.

}

export interface StatusBarData {
notifications: Notification[];

@echolayer-dev-karl echolayer-dev-karl Bot Apr 25, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define a type for the elements of the 'notifications' array in the 'StatusBarData' interface to ensure consistency and type safety.

}
Loading