Skip to content

Commit

Permalink
Refactor: Enhance file handling and code editing functionality (#2646)
Browse files Browse the repository at this point in the history
* refactor: Enhance file handling and code editing functionality

# PR Summary
**refactor: Enhance file handling and code editing functionality**

## PR Description

This pull request includes improvements to file handling, error management, and code editing functionality across multiple files. The changes enhance the robustness, security, and user experience of the application.

### Changes in `listen.py`

1. **Imports and Error Handling**:
   - Removed `warnings` import and its usage with `litellm`.
   - More consistent use of `JSONResponse` and `HTTPException` for error handling.

2. **WebSocket Endpoint (`/ws`)**:
   - Simplified logic for handling events using a single `isinstance` check.

3. **New Endpoint**:
   - Added `/api/save-file` POST endpoint for saving file contents.
   - Implemented checks for agent state before allowing file edits.

4. **Code Style and Organization**:
   - Improved code formatting and organization.
   - Refactored some functions for better readability and consistency.

### Changes in `fileService.ts`

1. **Error Handling**:
   - Added try-catch blocks to all functions for better error handling and logging.

2. **Input Sanitization**:
   - Implemented `encodeURIComponent()` for file names and paths in API requests.

3. **Type Checking**:
   - Added type checks for API responses to ensure data format consistency.

4. **File Upload Improvement**:
   - Refactored `uploadFiles()` to use `Array.from(files)` instead of a for loop.

5. **New Functionality**:
   - Added `saveFile()` function to allow saving file content to a specified path.

### Changes in `CodeEditor.tsx`

1. **New Dependencies**:
   - Added imports for state management, UI components, and file operations.

2. **State Management**:
   - Introduced new state variables for tracking save status and last saved time.
   - Implemented Redux state management for code and agent state.

3. **UI Enhancements**:
   - Added a save button with dynamic colors based on save status.
   - Implemented a save notification system.
   - Added a "Last saved" timestamp display.

4. **File Saving Functionality**:
   - Implemented complete file saving feature with error handling and user feedback.

5. **Code Structure**:
   - Improved structure with additional hooks and memoized values for optimization.

### Testing Performed
- Manually tested new file saving functionality.
- Verified error handling and user feedback mechanisms.
- Checked integration between backend (`listen.py`) and frontend (`fileService.ts`, `CodeEditor.tsx`).

### Next Steps
- Conduct thorough testing of the file saving feature across different scenarios.
- Update documentation to reflect new file handling capabilities.
- Consider adding unit tests for new functions and components.

* Added Docstrings back

Added Docstrings back

* Fix

# Allow Code Editing in AWAITING_USER_INPUT State

## Description
This pull request extends the functionality of the code editor to allow editing when the agent is in the AWAITING_USER_INPUT state, in addition to the existing PAUSED and FINISHED states.

## Changes
1. Backend (`listen.py`):
   - Updated the `save_file` function to allow saving when the agent state is AWAITING_USER_INPUT.

2. Frontend (`CodeEditor.tsx`):
   - Modified the `isEditingAllowed` condition to include the AWAITING_USER_INPUT state.

## Files Changed
- `listen.py`
- `CodeEditor.tsx`

## Testing
- Verified that the save button appears when the agent is in the AWAITING_USER_INPUT state.
- Tested saving files in all three allowed states (PAUSED, FINISHED, AWAITING_USER_INPUT).
- Ensured that saving is still prohibited in other agent states.

## Additional Notes
This change improves the user experience by allowing code edits while the agent is waiting for user input, which is a common scenario in interactive coding sessions.

* Add internationalization for 'File saved successfully' message

# Add internationalization for 'File saved successfully' message

## Description
This PR adds internationalization support for the "File saved successfully" message in the CodeEditor component. It updates the translation.json file to include translations for multiple languages and modifies the CodeEditor.tsx file to use the new translation key.

## Changes
1. Updated `translation.json`:
   - Added a new key `CODE_EDITOR$FILE_SAVED_SUCCESSFULLY` with translations for multiple languages.
   - Ensured the file structure supports multiple languages per key.

2. Modified `CodeEditor.tsx`:
   - Updated the success message to use the new translation key.
   - Applied the translation to both the toast notification and the on-screen notification.

## Why
These changes improve the user experience for non-English speakers by providing localized feedback when a file is successfully saved. This aligns with our goal of making the application more accessible to a global audience.

## How to Test
1. Change the application language to different supported languages.
2. Open the CodeEditor, make changes to a file, and save it.
3. Verify that the "File saved successfully" message appears in the correct language for both the toast and on-screen notifications.

## Additional Notes
Please pay special attention to the structure of the translation.json file to ensure it follows our established patterns for internationalization.

* Add toast notifications for error handling in fileService

# Add toast notifications for error handling in fileService

## Description
This PR enhances the error handling in the `fileService.ts` file by adding toast notifications for user feedback. It maintains the existing console error logging for debugging purposes while improving the user experience by providing visible error messages in the UI.

## Changes
- Added import for the toast utility
- Implemented toast.error() calls in catch blocks for all file operations
- Kept console.error() calls for detailed logging
- Updated error messages to be more user-friendly

## Files Changed
- `src/services/fileService.ts`

## Testing
- Tested all file operations (select, upload, list, save) to ensure proper error handling
- Verified that toast notifications appear when errors are simulated
- Confirmed that console errors are still logged for debugging

## Additional Notes
This change improves error visibility for users without altering the underlying error handling logic. It should make troubleshooting easier for both users and developers.

* Add file path safety check and improve error handling in file services

# Add file path safety check and improve error handling in file services

## Description
This PR enhances the `fileService.ts` by adding a safety check for file paths in the `saveFile` function and improves error handling across all file operations. It also includes new translations for various file-related error messages.

## Changes
1. Updated `src/services/fileService.ts`:
   - Added a validation check for file paths in the saveFile function
   - Improved error handling for all file operations (select, upload, list, save)
   - Implemented toast error messages with translation support

2. Updated `src/i18n/translations.json`:
   - Added new translation keys for file service error messages:
     - FILE_SERVICE$SELECT_FILE_ERROR
     - FILE_SERVICE$UPLOAD_FILES_ERROR
     - FILE_SERVICE$LIST_FILES_ERROR
     - FILE_SERVICE$SAVE_FILE_ERROR
     - FILE_SERVICE$INVALID_FILE_PATH

## Files Changed
- `src/services/fileService.ts`
- `src/i18n/translations.json`

## Key Implementation Details
```typescript
export async function saveFile(filePath: string, content: string): Promise<void> {
  const { t } = useTranslation();
  if (!filePath || filePath.includes('..')) {
    toast.error(t(I18nKey.FILE_SERVICE$INVALID_FILE_PATH));
    throw new Error('Invalid file path');
  }
  try {
    // Existing implementation...
  } catch (error) {
    console.error('Error saving file:', error);
    toast.error(t(I18nKey.FILE_SERVICE$SAVE_FILE_ERROR), 'File Save Error');
    throw error;
  }
}
```

## Testing
- Verified that the saveFile function rejects invalid file paths (empty or containing '..')
- Confirmed that appropriate error messages are displayed using toast notifications for all file operations
- Tested with different languages to ensure translated messages appear correctly

## Security Implications
The file path check in saveFile enhances security by preventing potential directory traversal attacks.

## Next Steps
- Consider adding similar safety checks to other file operations if applicable
- Ensure thorough testing of error scenarios across all supported languages

* Add docstrings to listen.py

# Add docstrings to listen.py

## Description
This PR adds comprehensive docstrings to all functions in the `listen.py` file. These additions improve code documentation, making the file more readable and maintainable for current and future developers.

## Changes
- Added docstrings to all functions in `listen.py`
- Docstrings follow the Google Python Style Guide format
- Included descriptions, parameters, return values, and potential exceptions for each function

## Files Changed
- `src/listen.py`

## Docstring Example
Here's an example of one of the added docstrings:

```python
@app.post('/api/save-file')
async def save_file(request: Request):
    """
    Save a file to the agent's runtime file store.

    This endpoint allows saving a file when the agent is in a paused, finished,
    or awaiting user input state. It checks the agent's state before proceeding
    with the file save operation.

    Args:
        request (Request): The incoming FastAPI request object.

    Returns:
        JSONResponse: A JSON response indicating the success of the operation.

    Raises:
        HTTPException:
            - 403 error if the agent is not in an allowed state for editing.
            - 400 error if the file path or content is missing.
            - 500 error if there's an unexpected error during the save operation.
    """
    # Function implementation...
```

## Impact
- Improved code readability and maintainability
- Better understanding of function purposes, inputs, outputs, and potential errors
- Easier onboarding for new developers working on this file
- Enhanced IDE support for function descriptions and parameter information

## Testing
- No functional changes were made, so existing tests should pass without modification
- Manual review of docstrings for accuracy and completeness is recommended

## Next Steps
- Consider adding similar docstrings to other files in the project for consistency
- Review the added docstrings to ensure they accurately describe the current functionality
- Update docstrings as needed when function implementations change in the future

## Additional Notes
The existing code structure and functionality remain unchanged. This PR focuses solely on improving documentation through the addition of docstrings.

* Revert exclude_list formatting and add docstrings in listen.py

# Revert exclude_list formatting and add docstrings in listen.py

## Description
This PR makes two main changes to the `listen.py` file:
1. Reverts the `exclude_list` in the `list_files` function to its original format, with each item on a separate line.
2. Adds comprehensive docstrings to all functions in the file.

These changes improve code readability, maintain consistency with project standards, and enhance documentation for better maintainability.

## Changes
1. Updated `opendevin/server/listen.py`:
   - Reverted `exclude_list` formatting in `list_files` function
   - Added docstrings to all functions

## Detailed Changes

### 1. Reverted exclude_list formatting
```python
exclude_list = (
    '.git',
    '.DS_Store',
    '.svn',
    '.hg',
    '.idea',
    '.vscode',
    '.settings',
    '.pytest_cache',
    '__pycache__',
    'node_modules',
    'vendor',
    'build',
    'dist',
    'bin',
    'logs',
    'log',
    'tmp',
    'temp',
    'coverage',
    'venv',
    'env',
)
```

### 2. Added docstrings (example)
```python
@app.get('/api/list-files')
def list_files(request: Request, path: str = '/'):
    """
    List files in the specified path.

    This function retrieves a list of files from the agent's runtime file store,
    excluding certain system and hidden files/directories.

    Args:
        request (Request): The incoming request object.
        path (str, optional): The path to list files from. Defaults to '/'.

    Returns:
        list: A list of file names in the specified path.

    Raises:
        HTTPException: If there's an error listing the files.
    """
    # Function implementation...
```

## Rationale
- Reverting `exclude_list` formatting maintains consistency with the project's coding style and ensures proper functioning of pre-commit hooks.
- Adding docstrings improves code documentation, making it easier for developers to understand and maintain the codebase.

## Impact
- Improved code readability and consistency
- Enhanced documentation for all functions in `listen.py`
- Easier onboarding for new developers
- Better IDE support for function descriptions and parameter information

## Testing
- No functional changes were made, so existing tests should pass without modification
- Manual review of the reverted `exclude_list` and new docstrings is recommended

## Additional Notes
- The existing code functionality remains unchanged
- All functions in `listen.py` now have detailed docstrings following the Google Python Style Guide format

## Next Steps
- Review the added docstrings to ensure they accurately describe the current functionality
- Consider adding similar docstrings to other files in the project for consistency
- Update docstrings as needed when function implementations change in the future

* made code reviewable

* fixed ruff issues

* Update listen.py docstrings

* final tweaks

* re-added encodedURIComponent in selectFile

---------

Co-authored-by: tobitege <[email protected]>
Co-authored-by: sp.wack <[email protected]>
  • Loading branch information
3 people authored Jul 2, 2024
1 parent 5e6fb61 commit bfa1de4
Show file tree
Hide file tree
Showing 8 changed files with 536 additions and 128 deletions.
82 changes: 0 additions & 82 deletions frontend/src/components/CodeEditor.tsx

This file was deleted.

2 changes: 1 addition & 1 deletion frontend/src/components/Workspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { initialState as initialCodeState } from "#/state/codeSlice";
import { RootState } from "#/store";
import { TabOption, TabType } from "#/types/TabOption";
import Browser from "./Browser";
import CodeEditor from "./CodeEditor";
import CodeEditor from "./file-explorer/CodeEditor";
import Planner from "./Planner";
import Jupyter from "./Jupyter";
import { getSettings } from "#/services/settings";
Expand Down
199 changes: 199 additions & 0 deletions frontend/src/components/file-explorer/CodeEditor.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
import React, { useMemo, useState, useCallback, useEffect } from "react";
import { useTranslation } from "react-i18next";
import { useDispatch, useSelector } from "react-redux";
import Editor, { Monaco } from "@monaco-editor/react";
import { Tab, Tabs, Button } from "@nextui-org/react";
import { VscCode, VscSave, VscCheck } from "react-icons/vsc";
import type { editor } from "monaco-editor";
import { I18nKey } from "#/i18n/declaration";
import { RootState } from "#/store";
import FileExplorer from "./FileExplorer";
import { setCode } from "#/state/codeSlice";
import toast from "#/utils/toast";
import { saveFile } from "#/services/fileService";
import AgentState from "#/types/AgentState";

function CodeEditor(): JSX.Element {
const { t } = useTranslation();
const dispatch = useDispatch();
const code = useSelector((state: RootState) => state.code.code);
const activeFilepath = useSelector((state: RootState) => state.code.path);
const agentState = useSelector(
(state: RootState) => state.agent.curAgentState,
);
const [saveStatus, setSaveStatus] = useState<
"idle" | "saving" | "saved" | "error"
>("idle");
const [showSaveNotification, setShowSaveNotification] = useState(false);
const [hasUnsavedChanges, setHasUnsavedChanges] = useState(false);
const [lastSavedContent, setLastSavedContent] = useState(code);

const selectedFileName = useMemo(() => {
const paths = activeFilepath.split("/");
return paths[paths.length - 1];
}, [activeFilepath]);

const isEditingAllowed = useMemo(
() =>
agentState === AgentState.INIT ||
agentState === AgentState.PAUSED ||
agentState === AgentState.FINISHED ||
agentState === AgentState.AWAITING_USER_INPUT,
[agentState],
);

useEffect(() => {
setSaveStatus("idle");
setHasUnsavedChanges(false);
setLastSavedContent(code);
}, [activeFilepath]);

useEffect(() => {
setHasUnsavedChanges(code !== lastSavedContent);
}, [code, lastSavedContent]);

const handleEditorChange = useCallback(
(value: string | undefined): void => {
if (value !== undefined && isEditingAllowed) {
dispatch(setCode(value));
setHasUnsavedChanges(true);
}
},
[dispatch, isEditingAllowed],
);

const handleEditorDidMount = useCallback(
(editor: editor.IStandaloneCodeEditor, monaco: Monaco): void => {
monaco.editor.defineTheme("my-theme", {
base: "vs-dark",
inherit: true,
rules: [],
colors: {
"editor.background": "#171717",
},
});

monaco.editor.setTheme("my-theme");
},
[],
);

const handleSave = useCallback(async (): Promise<void> => {
if (saveStatus === "saving" || !isEditingAllowed) return;

setSaveStatus("saving");

try {
await saveFile(activeFilepath, code);
setSaveStatus("saved");
setShowSaveNotification(true);
setLastSavedContent(code);
setHasUnsavedChanges(false);
setTimeout(() => setShowSaveNotification(false), 2000);
toast.success(
"file-save-success",
t(I18nKey.CODE_EDITOR$FILE_SAVED_SUCCESSFULLY),
);
} catch (error) {
setSaveStatus("error");
if (error instanceof Error) {
toast.error(
"file-save-error",
`${t(I18nKey.CODE_EDITOR$FILE_SAVE_ERROR)}: ${error.message}`,
);
} else {
toast.error("file-save-error", t(I18nKey.CODE_EDITOR$FILE_SAVE_ERROR));
}
}
}, [
saveStatus,
activeFilepath,
code,
isEditingAllowed,
t,
hasUnsavedChanges,
]);

const getSaveButtonColor = () => {
switch (saveStatus) {
case "saving":
return "bg-yellow-600";
case "saved":
return "bg-green-600";
case "error":
return "bg-red-600";
default:
return "bg-blue-600";
}
};

return (
<div className="flex h-full w-full bg-neutral-900 transition-all duration-500 ease-in-out relative">
<FileExplorer />
<div className="flex flex-col min-h-0 w-full">
<div className="flex justify-between items-center border-b border-neutral-600 mb-4">
<Tabs
disableCursorAnimation
classNames={{
base: "w-full",
tabList:
"w-full relative rounded-none bg-neutral-900 p-0 border-divider",
cursor: "w-full bg-neutral-600 rounded-none",
tab: "max-w-fit px-4 h-[36px]",
tabContent: "group-data-[selected=true]:text-white",
}}
aria-label={t(I18nKey.CODE_EDITOR$OPTIONS)}
>
<Tab
key={selectedFileName}
title={selectedFileName || t(I18nKey.CODE_EDITOR$EMPTY_MESSAGE)}
/>
</Tabs>
{selectedFileName && hasUnsavedChanges && (
<div className="flex items-center mr-2">
<Button
onClick={handleSave}
className={`${getSaveButtonColor()} text-white transition-colors duration-300 mr-2`}
size="sm"
startContent={<VscSave />}
disabled={saveStatus === "saving" || !isEditingAllowed}
>
{saveStatus === "saving"
? t(I18nKey.CODE_EDITOR$SAVING_LABEL)
: t(I18nKey.CODE_EDITOR$SAVE_LABEL)}
</Button>
</div>
)}
</div>
<div className="flex grow items-center justify-center">
{!selectedFileName ? (
<div className="flex flex-col items-center text-neutral-400">
<VscCode size={100} />
{t(I18nKey.CODE_EDITOR$EMPTY_MESSAGE)}
</div>
) : (
<Editor
height="100%"
path={selectedFileName.toLowerCase()}
defaultValue=""
value={code}
onMount={handleEditorDidMount}
onChange={handleEditorChange}
options={{ readOnly: !isEditingAllowed }}
/>
)}
</div>
</div>
{showSaveNotification && (
<div className="absolute bottom-4 left-1/2 transform -translate-x-1/2">
<div className="bg-green-500 text-white px-4 py-2 rounded-lg flex items-center justify-center animate-pulse">
<VscCheck className="mr-2 text-xl" />
<span>{t(I18nKey.CODE_EDITOR$FILE_SAVED_SUCCESSFULLY)}</span>
</div>
</div>
)}
</div>
);
}

export default CodeEditor;
2 changes: 1 addition & 1 deletion frontend/src/components/file-explorer/FileExplorer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ function FileExplorer() {
</div>
<div className="overflow-auto flex-grow">
<div style={{ display: isHidden ? "none" : "block" }}>
<ExplorerTree files={files} defaultOpen />
<ExplorerTree files={files} />
</div>
</div>
</div>
Expand Down
Loading

0 comments on commit bfa1de4

Please sign in to comment.