fix: rename symbol, dynamic add of user symbols, docs on autocomplete for user syms

Cargo.toml

@@ -4,7 +4,7 @@ resolver = "2"
 
 [package]
 name = "forth-lsp"
-version = "0.3.0"
+version = "0.3.1"
 edition = "2021"
 license = "MIT"
 description = "LSP for the Forth programming language"

README.md

@@ -2,39 +2,37 @@
 
 [![CI](https://github.com/AlexanderBrevig/forth-lsp/actions/workflows/ci.yml/badge.svg)](https://github.com/AlexanderBrevig/forth-lsp/actions/workflows/ci.yml)
 
-`forth-lsp` is an implementation of the [Language Server Protocol](https://microsoft.github.io/language-server-protocol/) for the [Forth](https://forth-standard.org/) programming language.
+A [Language Server Protocol](https://microsoft.github.io/language-server-protocol/) implementation for [Forth](https://forth-standard.org/), bringing modern IDE features to Forth development.
 
-I like forth, and I love [helix](https://github.com/helix-editor/helix)!
-This project is a companion to [tree-sitter-forth](https://github.com/AlexanderBrevig/tree-sitter-forth) in order to make forth barable on helix :)
+## Features
 
-Currently this simple LSP supports `Hover`, `Completion` and `GotoDefinition`.
+- **Hover** - View documentation for built-in words and user-defined functions
+- **Completion** - Auto-complete for built-in words and your definitions
+- **Go to Definition** - Jump to where words are defined
+- **Find References** - Find all usages of a word
+- **Rename** - Rename symbols across your workspace
+- **Document Symbols** - Outline view of word definitions in current file
+- **Workspace Symbols** - Search for definitions across all files
+- **Signature Help** - View parameter information while typing
+- **Diagnostics** - Real-time error detection for undefined words
 
-[Issues](https://github.com/AlexanderBrevig/forth-lsp/issues) and [PRs](https://github.com/AlexanderBrevig/forth-lsp/pulls) are very welcome!
-
-## Install
+## Installation
 
 ```shell
 cargo install forth-lsp
 ```
 
-You can now configure your editor to use this LSP.
-
-## Development
+Then configure your editor to use `forth-lsp`. Works with any LSP-compatible editor (VS Code, Neovim, Helix, Emacs, etc.).
 
-This is a Cargo workspace containing:
+## Contributing
 
-- `forth-lsp` - The main LSP server
-- `lib/forth-lexer` - The Forth lexer/tokenizer library
+[Issues](https://github.com/AlexanderBrevig/forth-lsp/issues) and [PRs](https://github.com/AlexanderBrevig/forth-lsp/pulls) welcome!
 
-### Testing
+### Development
 
 ```shell
-# Test all workspace members (both forth-lsp and forth-lexer)
+# Run tests
 cargo test --workspace
-
-# Or use the convenient alias
+# or
 cargo t
-
-# Test only the main forth-lsp package
-cargo test
 ```

lib/forth-lexer/src/parser.rs

@@ -176,10 +176,11 @@ impl<'a> Lexer<'a> {
             self.read_char();
         }
 
+        let end = self.position.min(self.raw.len());
         Data {
             start,
-            end: self.position,
-            value: &self.raw[start..self.position],
+            end,
+            value: &self.raw[start..end],
         }
     }
 
@@ -188,10 +189,11 @@ impl<'a> Lexer<'a> {
         while !self.ch.is_whitespace() && self.ch != '\0' {
             self.read_char();
         }
+        let end = self.position.min(self.raw.len());
         Data {
             start,
-            end: self.position,
-            value: &self.raw[start..self.position],
+            end,
+            value: &self.raw[start..end],
         }
     }
 
@@ -207,10 +209,11 @@ impl<'a> Lexer<'a> {
         {
             self.read_char();
         }
+        let end = self.position.min(self.raw.len());
         Data {
             start,
-            end: self.position,
-            value: &self.raw[start..self.position],
+            end,
+            value: &self.raw[start..end],
         }
     }
 

src/main.rs

@@ -7,11 +7,13 @@ use crate::prelude::*;
 use crate::utils::definition_index::DefinitionIndex;
 use crate::utils::handlers::notification_did_change::handle_did_change_text_document;
 use crate::utils::handlers::notification_did_open::handle_did_open_text_document;
+use crate::utils::handlers::notification_did_save::handle_did_save_text_document;
 use crate::utils::handlers::request_completion::handle_completion;
 use crate::utils::handlers::request_document_symbols::handle_document_symbols;
 use crate::utils::handlers::request_find_references::handle_find_references;
 use crate::utils::handlers::request_goto_definition::handle_goto_definition;
 use crate::utils::handlers::request_hover::handle_hover;
+use crate::utils::handlers::request_prepare_rename::handle_prepare_rename;
 use crate::utils::handlers::request_rename::handle_rename;
 use crate::utils::handlers::request_signature_help::handle_signature_help;
 use crate::utils::handlers::request_workspace_symbols::handle_workspace_symbols;
@@ -87,6 +89,9 @@ fn main_loop(connection: Connection, params: serde_json::Value) -> Result<()> {
                 if handle_find_references(&request, &connection, &mut files, &def_index).is_ok() {
                     continue;
                 }
+                if handle_prepare_rename(&request, &connection, &files, &def_index).is_ok() {
+                    continue;
+                }
                 if handle_rename(&request, &connection, &mut files, &def_index).is_ok() {
                     continue;
                 }
@@ -127,6 +132,17 @@ fn main_loop(connection: Connection, params: serde_json::Value) -> Result<()> {
                 {
                     continue;
                 }
+                if handle_did_save_text_document(
+                    &notification,
+                    &connection,
+                    &mut files,
+                    &mut def_index,
+                    &data,
+                )
+                .is_ok()
+                {
+                    continue;
+                }
             }
         }
     }
@@ -147,7 +163,9 @@ fn load_dir(
                     let raw_content = fs::read(entry)?;
                     let content = String::from_utf8_lossy(&raw_content);
                     let rope = Rope::from_str(&content);
-                    files.insert(entry.to_string(), rope);
+                    // Convert path to URI to match DidOpen/DidChange format
+                    let file_uri = format!("file://{}", entry);
+                    files.insert(file_uri, rope);
                 }
             }
         }

src/utils/definition_index.rs

@@ -65,17 +65,15 @@ impl DefinitionIndex {
                     definition_token_indices.insert(data.start);
                 }
 
-                let Some((name, _selection_start, _selection_end)) =
+                let Some((name, selection_start, selection_end)) =
                     extract_word_name_with_range(result, 1)
                 else {
                     continue;
                 };
 
-                let tok = Token::Illegal(Data::new(0, 0, ""));
-                let begin = result.first().unwrap_or(&tok).get_data();
-                let end = result.last().unwrap_or(&tok).get_data();
-
-                let range = data_range_from_to(begin, end, rope);
+                // Use the name's range, not the entire definition block
+                let name_data = Data::new(selection_start, selection_end, "");
+                let range = data_range_from_to(&name_data, &name_data, rope);
 
                 // Store with lowercase key for case-insensitive lookup
                 self.definitions
@@ -115,8 +113,8 @@ impl DefinitionIndex {
 
                         let name = name_data.value.to_string();
 
-                        // Create range spanning from the defining word to the name
-                        let range = data_range_from_to(defining_word_data, name_data, rope);
+                        // Use only the name's range, not including the defining word
+                        let range = name_data.to_range(rope);
 
                         // Store with lowercase key for case-insensitive lookup
                         self.definitions
@@ -167,8 +165,15 @@ impl DefinitionIndex {
         let mut locations = Vec::new();
 
         if let Some(defs) = self.definitions.get(&word.to_lowercase()) {
-            for (file_path, range) in defs {
-                if let Ok(uri) = Url::from_file_path(file_path) {
+            for (file_path_or_uri, range) in defs {
+                // Try parsing as URI first (file:// scheme), then fall back to file path
+                let uri = if file_path_or_uri.starts_with("file://") {
+                    Url::parse(file_path_or_uri).ok()
+                } else {
+                    Url::from_file_path(file_path_or_uri).ok()
+                };
+
+                if let Some(uri) = uri {
                     locations.push(Location { uri, range: *range });
                 }
             }
@@ -182,8 +187,15 @@ impl DefinitionIndex {
         let mut locations = Vec::new();
 
         if let Some(refs) = self.references.get(&word.to_lowercase()) {
-            for (file_path, range) in refs {
-                if let Ok(uri) = Url::from_file_path(file_path) {
+            for (file_path_or_uri, range) in refs {
+                // Try parsing as URI first (file:// scheme), then fall back to file path
+                let uri = if file_path_or_uri.starts_with("file://") {
+                    Url::parse(file_path_or_uri).ok()
+                } else {
+                    Url::from_file_path(file_path_or_uri).ok()
+                };
+
+                if let Some(uri) = uri {
                     locations.push(Location { uri, range: *range });
                 }
             }
@@ -493,7 +505,8 @@ mod tests {
         let defs = index.find_definitions("DATE");
         assert_eq!(defs.len(), 1);
         assert_eq!(defs[0].range.start.line, 0);
-        assert_eq!(defs[0].range.start.character, 0);
+        // Range should be for "DATE" (starts at character 9), not "VARIABLE"
+        assert_eq!(defs[0].range.start.character, 9);
     }
 
     #[test]

src/utils/handlers/common.rs

@@ -3,47 +3,36 @@ use lsp_types::{Position, TextDocumentIdentifier, TextDocumentPositionParams};
 
 /// Extracted position information from a TextDocumentPositionParams
 pub struct ExtractedPosition {
-    pub file_path: String,
+    pub file_uri: String,
     pub line: u32,
     pub character: u32,
 }
 
 impl ExtractedPosition {
-    /// Extract file path and position from TextDocumentPositionParams
+    /// Extract file URI and position from TextDocumentPositionParams
     pub fn from_text_document_position(params: &TextDocumentPositionParams) -> Result<Self> {
-        let file_path = params
-            .text_document
-            .uri
-            .to_file_path()
-            .map_err(|_| Error::Generic("Invalid file path".to_string()))?
-            .to_string_lossy()
-            .to_string();
+        let file_uri = params.text_document.uri.to_string();
 
         Ok(ExtractedPosition {
-            file_path,
+            file_uri,
             line: params.position.line,
             character: params.position.character,
         })
     }
 
-    /// Extract file path and position from components
+    /// Extract file URI and position from components
     pub fn from_parts(text_document: &TextDocumentIdentifier, position: &Position) -> Result<Self> {
-        let file_path = text_document
-            .uri
-            .to_file_path()
-            .map_err(|_| Error::Generic("Invalid file path".to_string()))?
-            .to_string_lossy()
-            .to_string();
+        let file_uri = text_document.uri.to_string();
 
         Ok(ExtractedPosition {
-            file_path,
+            file_uri,
             line: position.line,
             character: position.character,
         })
     }
 
     /// Format position for logging
     pub fn format(&self) -> String {
-        format!("{}:{}:{}", self.file_path, self.line, self.character)
+        format!("{}:{}:{}", self.file_uri, self.line, self.character)
     }
 }

src/utils/handlers/mod.rs

@@ -4,11 +4,13 @@ use crate::prelude::*;
 pub mod common;
 pub mod notification_did_change;
 pub mod notification_did_open;
+pub mod notification_did_save;
 pub mod request_completion;
 pub mod request_document_symbols;
 pub mod request_find_references;
 pub mod request_goto_definition;
 pub mod request_hover;
+pub mod request_prepare_rename;
 pub mod request_rename;
 pub mod request_signature_help;
 pub mod request_workspace_symbols;

src/utils/handlers/notification_did_change.rs

@@ -22,18 +22,30 @@ pub fn handle_did_change_text_document(
     {
         Ok(params) => {
             let file_uri = params.text_document.uri.to_string();
+            log_debug!("DidChange for file: {}", file_uri);
             let rope = files
                 .get_mut(&file_uri)
                 .expect("Must be able to get rope for lang");
+            log_debug!(
+                "Processing {} content changes",
+                params.content_changes.len()
+            );
             for change in params.content_changes {
+                log_debug!(
+                    "Change range: {:?}, text length: {}",
+                    change.range,
+                    change.text.len()
+                );
                 let range = change.range.unwrap_or_default();
                 let start =
                     rope.line_to_char(range.start.line as usize) + range.start.character as usize;
                 let end = rope.line_to_char(range.end.line as usize) + range.end.character as usize;
                 rope.remove(start..end);
                 rope.insert(start, change.text.as_str());
             }
+            log_debug!("After changes, rope has {} chars", rope.len_chars());
             // Update definition index for the changed file
+            log_debug!("Updating definition index for: {}", file_uri);
             def_index.update_file(&file_uri, rope);
 
             // Publish diagnostics for the changed file

src/utils/handlers/notification_did_open.rs

@@ -21,6 +21,7 @@ pub fn handle_did_open_text_document(
     match cast_notification::<lsp_types::notification::DidOpenTextDocument>(notification.clone()) {
         Ok(params) => {
             let file_uri = params.text_document.uri.to_string();
+            log_debug!("DidOpen for file: {}", file_uri);
             if let std::collections::hash_map::Entry::Vacant(e) = files.entry(file_uri.clone()) {
                 let rope = Rope::from_str(params.text_document.text.as_str());
                 // Update index for newly opened file