-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat(vscode): add at feature with AT file implementation in VSCode #3602
base: main
Are you sure you want to change the base?
Conversation
5244117
to
ccad4a3
Compare
let label: string; | ||
|
||
if (filepath.kind === "git") { | ||
label = filepath.filepath; | ||
} else { | ||
const workspaceFolder = workspace.getWorkspaceFolder(uri); | ||
if (workspaceFolder) { | ||
label = path.relative(workspaceFolder.uri.fsPath, uri.fsPath); | ||
} else { | ||
label = path.basename(uri.fsPath); | ||
} | ||
} |
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.
Please try to use method workspace.asRelativePath
:
let label: string; | |
if (filepath.kind === "git") { | |
label = filepath.filepath; | |
} else { | |
const workspaceFolder = workspace.getWorkspaceFolder(uri); | |
if (workspaceFolder) { | |
label = path.relative(workspaceFolder.uri.fsPath, uri.fsPath); | |
} else { | |
label = path.basename(uri.fsPath); | |
} | |
} | |
const label = worksapce.asRelativePath(uri); |
export function extractTextFromRange(document: TextDocument, range: LineRange | PositionRange): string { | ||
if (typeof range.start === "number" && typeof range.end === "number") { | ||
const startLine = range.start - 1; | ||
const endLine = range.end - 1; | ||
let selectedText = ""; | ||
|
||
for (let i = startLine; i <= endLine; i++) { | ||
if (i < 0 || i >= document.lineCount) { | ||
continue; | ||
} | ||
selectedText += document.lineAt(i).text + "\n"; | ||
} | ||
return selectedText; | ||
} | ||
|
||
if (typeof range.start === "object" && typeof range.end === "object") { | ||
const startPos = new VSCodePosition(range.start.line - 1, range.start.character - 1); | ||
const endPos = new VSCodePosition(range.end.line - 1, range.end.character - 1); | ||
const selectedRange = new VSCodeRange(startPos, endPos); | ||
return document.getText(selectedRange); | ||
} | ||
|
||
return ""; | ||
} |
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.
Can we reuse chatPanelLocationToVSCodeRange
to convert range: LineRange | PositionRange
to VSCodeRange?
By reusing thiat, there is no need to extract extractTextFromRange
.
this.logger.info(`No query provided, listing ${openTabs.length} opened editors.`); | ||
|
||
return openTabs | ||
.filter((tab) => tab.uri.scheme === "file") |
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.
Why is this filter required?
.map((tab) => ({ | ||
uri: (tab.input as TabInputText).uri, | ||
label: tab.label, | ||
})), |
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.
I think this map is not required here. You can directly map it with uriToListFileItem
.
.map((tab) => uriToListFileItem(tab.uri, this.gitProvider)); | ||
} | ||
|
||
const globPattern = `**/${query}*`; |
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.
Should we validate the query string before injecting it into the glob pattern to ensure **/${query}*
is safe? For example, what if the query string contains *
or other special characters?
}, | ||
|
||
readFileContent: async (info: FileRange): Promise<string | null> => { | ||
if (info.range) { |
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.
There's no need for an if
branch here. document.getText()
can accept a range or undefined
as parameters.
} catch (error) { | ||
this.logger.error("Failed to get file content by range:", error); | ||
return null; | ||
} |
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.
No need to catch error, or just rethrow it.
not git repo
in git repo