- 
                Notifications
    
You must be signed in to change notification settings  - Fork 8
 
Feat/lsp api #6
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.
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
base: main
Are you sure you want to change the base?
Feat/lsp api #6
Conversation
| 
           /gemini summary  | 
    
| 
           Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services.  | 
    
| 
           /gemini review  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
The pull request introduces several new features, including LSP API integration, improved context handling, and enhanced suggestion generation. The changes are substantial and touch various parts of the codebase. Overall, the code appears well-structured, but there are some areas that could benefit from further refinement and clarification.
Summary of Findings
- Error Handling in 
with_token: Thewith_tokenfunction inlua/nes/api.luauses a callback for error handling, but the error message is a simple string. Consider providing more context or a structured error object for better debugging. - Code Duplication: The code for removing specific lines (e.g., 
<next-version>, ```) is duplicated inlua/nes/core.luaand `lua/nes/context.lua`. This logic should be extracted into a utility function to improve maintainability. - Clarity of 
ContextClass Fields: The purpose and usage of fields likeoriginal_code,edits, andcurrent_versionin thenes.Contextclass could be better documented to enhance understanding and maintainability. - Potential Performance Bottleneck: The 
generate_editsfunction inlua/nes/core.luais called within avim.scheduleblock. Ensure that this operation does not block the main thread, especially for large files or complex edits. 
Merge Readiness
The pull request introduces significant functionality and is generally well-structured. However, addressing the identified issues related to error handling, code duplication, documentation, and potential performance bottlenecks would improve the code's robustness and maintainability. I am unable to directly approve the pull request, and other reviewers should review and approve this code before merging. I recommend addressing the high and medium severity issues before merging.
| 
           /gemini summary  | 
    
          Summary of ChangesThis pull request introduces Language Server Protocol (LSP) support for the  Highlights
 ChangelogClick here to see the changelog
 Activity
  | 
    
22a3713    to
    938e302      
    Compare
  
    d240f24    to
    c2aab54      
    Compare
  
    d6a5694    to
    44d06c4      
    Compare
  
    | 
           This looks really promising 🚀  | 
    
No description provided.