Skip to content

Commit c0f3fa1

Browse files
authored
Disable completions due to issues with non-ascii chars (#1917)
<!-- ELLIPSIS_HIDDEN --> > [!IMPORTANT] > Disables completion feature due to non-ASCII character issues and updates position handling for multibyte characters in `position_utils.rs`. > > - **Behavior**: > - Disables completion feature in `completion.rs` due to issues with non-ASCII characters, specifically Chinese/multibyte characters. > - Returns `None` for completion requests in `Completion::run()`. > - **Position Handling**: > - Updates `is_first_inside_block()`, `get_word_at_position()`, and `get_symbol_before_position()` in `position_utils.rs` to convert character positions to byte offsets for accurate slicing. > - Ensures character positions are clamped to line length to prevent out-of-bounds errors. > - **Misc**: > - Comments in `completion.rs` suggest conditions for re-enabling completions, including testing on Windows and modifying position handling for multibyte characters. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=BoundaryML%2Fbaml&utm_source=github&utm_medium=referral)<sup> for 4467798. You can [customize](https://app.ellipsis.dev/BoundaryML/settings/summaries) this summary. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
1 parent 4b7075a commit c0f3fa1

2 files changed

Lines changed: 125 additions & 71 deletions

File tree

engine/language_server/src/baml_project/position_utils.rs

Lines changed: 73 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::cmp;
55
const MAX_SAFE_CHAR: u32 = std::u32::MAX; // You can adjust this to mimic your TS MAX_SAFE_VALUE_i32
66

77
/// Returns the full range of the document, from the beginning (line 0, character 0)
8-
/// to the end (last line with a very large character position).
8+
/// to the "end" (last line with a very large character position).
99
pub fn full_document_range(contents: &str) -> Range {
1010
// Compute the number of lines. If the text is empty, assume one line.
1111
let line_count = if contents.is_empty() {
@@ -42,15 +42,23 @@ pub fn get_current_line<'a>(contents: &'a str, line: u32) -> &'a str {
4242
/// The logic is as follows:
4343
/// - If the trimmed current line is empty, return true.
4444
/// - Otherwise, take the substring _up to_ the given position and check if the last
45-
/// word (using a `\w+` search) ends exactly at the position.
45+
/// "word" (using a `\w+` search) ends exactly at the position.
4646
pub fn is_first_inside_block(position: &Position, current_line: &str) -> bool {
4747
if current_line.trim().is_empty() {
4848
return true;
4949
}
5050

51-
// Ensure we don’t slice past the length of the current line.
52-
let pos = cmp::min(position.character as usize, current_line.len());
53-
let string_til_position = &current_line[..pos];
51+
// Ensure we don't slice past the length of the current line, using character counts.
52+
let char_count_in_line = current_line.chars().count();
53+
let clamped_char_pos = cmp::min(position.character as usize, char_count_in_line);
54+
55+
// Convert character position to byte offset for slicing.
56+
let byte_offset_at_clamped_char_pos = current_line
57+
.char_indices()
58+
.nth(clamped_char_pos)
59+
.map_or(current_line.len(), |(idx, _)| idx);
60+
61+
let string_til_position = &current_line[..byte_offset_at_clamped_char_pos];
5462

5563
// Find the first occurrence of a word.
5664
let re = Regex::new(r"\w+").unwrap();
@@ -71,21 +79,37 @@ pub fn is_first_inside_block(position: &Position, current_line: &str) -> bool {
7179
/// If no non-word boundary is found after the position, an empty string is returned.
7280
pub fn get_word_at_position(contents: &str, position: &Position) -> String {
7381
let current_line = get_current_line(contents, position.line);
74-
let line_len = current_line.len();
82+
if current_line.is_empty() {
83+
return "".to_string();
84+
}
7585

76-
// Clamp position.character to the current line length.
77-
let pos = cmp::min(position.character as usize, line_len);
86+
let char_count_in_line = current_line.chars().count();
87+
// `position.character` is a 0-indexed character offset. Clamp it.
88+
let clamped_char_idx = cmp::min(position.character as usize, char_count_in_line);
89+
90+
// Part 1: Search backward to find the start of the word.
91+
// Slice for backward search extends up to character `clamped_char_idx + 1` (exclusive).
92+
let char_len_for_backward_search = cmp::min(clamped_char_idx + 1, char_count_in_line);
93+
let byte_len_for_backward_search_slice = current_line
94+
.char_indices()
95+
.nth(char_len_for_backward_search)
96+
.map_or(current_line.len(), |(idx, _)| idx);
97+
let text_for_backward_search = &current_line[..byte_len_for_backward_search_slice];
7898

7999
// Search backward from position.character + 1 using a regex for the last non-whitespace sequence.
80100
let re_begin = Regex::new(r"\S+$").unwrap();
81-
let slice_end = cmp::min(pos + 1, line_len);
82-
let substring_before = &current_line[..slice_end];
83-
let beginning = if let Some(mat) = re_begin.find(substring_before) {
84-
mat.start()
85-
} else {
86-
return "".to_string();
101+
let word_start_byte_idx = match re_begin.find(text_for_backward_search) {
102+
Some(mat) => mat.start(),
103+
None => return "".to_string(),
87104
};
88105

106+
// Convert `clamped_char_idx` (character index) to a byte index for the LSP `position.character`.
107+
// This `pos` will be used as the starting point for the forward search.
108+
let pos = current_line
109+
.char_indices()
110+
.nth(clamped_char_idx)
111+
.map_or(current_line.len(), |(idx, _)| idx);
112+
89113
// Search forward from position.character for the first non-word character.
90114
let re_end = Regex::new(r"\W").unwrap();
91115
let substring_after = &current_line[pos..];
@@ -97,8 +121,8 @@ pub fn get_word_at_position(contents: &str, position: &Position) -> String {
97121

98122
let word_end = pos + end;
99123

100-
if beginning <= word_end && word_end <= current_line.len() {
101-
current_line[beginning..word_end].to_string()
124+
if word_start_byte_idx <= word_end && word_end <= current_line.len() {
125+
current_line[word_start_byte_idx..word_end].to_string()
102126
} else {
103127
"".to_string()
104128
}
@@ -107,19 +131,44 @@ pub fn get_word_at_position(contents: &str, position: &Position) -> String {
107131
/// Returns the symbol (a single character) immediately preceding the given position.
108132
/// If the position is at the start of the line, an empty string is returned.
109133
pub fn get_symbol_before_position(contents: &str, position: &Position) -> String {
110-
if position.character == 0 {
111-
return "".to_string();
112-
}
113134
let current_line = get_current_line(contents, position.line);
114-
let pos = cmp::min(position.character as usize, current_line.len());
115-
if pos == 0 {
135+
// position.character is a 0-indexed character offset.
136+
let char_cursor_pos = position.character as usize;
137+
138+
if char_cursor_pos == 0 {
116139
return "".to_string();
117140
}
118-
// This simple slicing works correctly if the text is ASCII.
119-
current_line[pos - 1..pos].to_string()
141+
142+
// Clamp the character cursor position against the actual number of characters in the line.
143+
// This ensures that if position.character is, for example, 5, but the line only has 3 chars,
144+
// we don't panic. We want the character at index (clamped_char_cursor_pos - 1).
145+
let num_chars_in_line = current_line.chars().count();
146+
147+
// If the effective cursor position is beyond the line's character length,
148+
// or if it's at the very beginning (char_cursor_pos == 0, handled above),
149+
// there's no valid preceding character to get by simple indexing from char_cursor_pos.
150+
// We are interested in the character at `char_cursor_pos - 1`.
151+
if char_cursor_pos > num_chars_in_line {
152+
// If cursor is effectively beyond the line, the "preceding" character would be the last one.
153+
// So we try to get char at num_chars_in_line - 1.
154+
if num_chars_in_line == 0 {
155+
return "".to_string(); // Empty line
156+
}
157+
return current_line
158+
.chars()
159+
.nth(num_chars_in_line - 1)
160+
.map_or("".to_string(), |ch| ch.to_string());
161+
}
162+
163+
// At this point, 0 < char_cursor_pos <= num_chars_in_line.
164+
// We want the character at index (char_cursor_pos - 1).
165+
current_line
166+
.chars()
167+
.nth(char_cursor_pos - 1)
168+
.map_or("".to_string(), |ch| ch.to_string())
120169
}
121170

122-
/// Computes the Position (line and character) corresponding to a given index in the documents text.
171+
/// Computes the Position (line and character) corresponding to a given index in the document's text.
123172
/// This mimics the TS implementation by iterating over each character up to the given index.
124173
pub fn get_position_from_index(document: &TextDocumentItem, index: usize) -> Position {
125174
let mut line: u32 = 0;

engine/language_server/src/server/api/requests/completion.rs

Lines changed: 52 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -20,54 +20,59 @@ impl SyncRequestHandler for Completion {
2020
_requester: &mut Requester,
2121
params: CompletionParams,
2222
) -> Result<Option<lsp_types::CompletionResponse>> {
23-
let url = params.text_document_position.text_document.uri;
24-
let path = url
25-
.to_file_path()
26-
.internal_error_msg("Could not convert URL to path")?;
23+
// TODO: Enable this only if you
24+
// 1. test on windows, with chinese characters
25+
// 2. Modify position_utils.rs to use byte offsets to account for chinese/multibyte characters
26+
// 3. Don't crash if you index into a string with a byte offset that is out of bounds
27+
// let url = params.text_document_position.text_document.uri;
28+
// let path = url
29+
// .to_file_path()
30+
// .internal_error_msg("Could not convert URL to path")?;
2731

28-
// Use the unified method to get or create the project
29-
let project = session
30-
.get_or_create_project(&path)
31-
.expect("Failed to get or create project");
32+
// // Use the unified method to get or create the project
33+
// let project = session
34+
// .get_or_create_project(&path)
35+
// .expect("Failed to get or create project");
3236

33-
let guard = project.lock().unwrap();
34-
let document_key =
35-
DocumentKey::from_url(&PathBuf::from(guard.root_path()), &url).internal_error()?;
36-
let doc = guard
37-
.baml_project
38-
.files
39-
.get(&document_key)
40-
.ok_or(anyhow::anyhow!(
41-
"File {} was not present in the project",
42-
document_key
43-
))
44-
.internal_error()?;
45-
let word = get_word_at_position(&doc.contents, &params.text_document_position.position);
46-
let cleaned_word = trim_line(&word);
47-
// let cleaned_word = word;
48-
let completions = match cleaned_word.as_str() {
49-
"_." => Some(vec![
50-
r#"role("system")"#,
51-
r#"role("assistant")"#,
52-
r#"role("user")"#,
53-
]),
54-
"ctx." => Some(vec![r#"output_format"#, r#"client"#]),
55-
"ctx.client." => Some(vec![r#"name"#, r#"provider"#]),
56-
_ => None,
57-
};
58-
Ok(completions.map(|completions| {
59-
let completion_list = CompletionList {
60-
is_incomplete: false,
61-
items: completions
62-
.into_iter()
63-
.map(|completion| CompletionItem {
64-
label: completion.to_string(),
65-
..CompletionItem::default()
66-
})
67-
.collect(),
68-
..CompletionList::default()
69-
};
70-
CompletionResponse::List(completion_list)
71-
}))
37+
// let guard = project.lock().unwrap();
38+
// let document_key =
39+
// DocumentKey::from_url(&PathBuf::from(guard.root_path()), &url).internal_error()?;
40+
// let doc = guard
41+
// .baml_project
42+
// .files
43+
// .get(&document_key)
44+
// .ok_or(anyhow::anyhow!(
45+
// "File {} was not present in the project",
46+
// document_key
47+
// ))
48+
// .internal_error()?;
49+
// let word = get_word_at_position(&doc.contents, &params.text_document_position.position);
50+
// let cleaned_word = trim_line(&word);
51+
// // let cleaned_word = word;
52+
// let completions = match cleaned_word.as_str() {
53+
// "_." => Some(vec![
54+
// r#"role("system")"#,
55+
// r#"role("assistant")"#,
56+
// r#"role("user")"#,
57+
// ]),
58+
// "ctx." => Some(vec![r#"output_format"#, r#"client"#]),
59+
// "ctx.client." => Some(vec![r#"name"#, r#"provider"#]),
60+
// _ => None,
61+
// };
62+
// Ok(completions.map(|completions| {
63+
// let completion_list = CompletionList {
64+
// is_incomplete: false,
65+
// items: completions
66+
// .into_iter()
67+
// .map(|completion| CompletionItem {
68+
// label: completion.to_string(),
69+
// ..CompletionItem::default()
70+
// })
71+
// .collect(),
72+
// ..CompletionList::default()
73+
// };
74+
// CompletionResponse::List(completion_list)
75+
// }))
76+
Ok(None)
7277
}
7378
}

0 commit comments

Comments
 (0)