Skip to content
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

Bash builtins don't show Enhanced Documentation Support #241927

Closed
joshspicer opened this issue Feb 25, 2025 · 11 comments
Closed

Bash builtins don't show Enhanced Documentation Support #241927

joshspicer opened this issue Feb 25, 2025 · 11 comments
Assignees
Labels
info-needed Issue requires more information from poster
Milestone

Comments

@joshspicer
Copy link
Member

joshspicer commented Feb 25, 2025

Testing #241749

Following 3: Enhanced Documentation Support from TPI.

Image
Version: 1.98.0-insider
Commit: ac0e8f0f32e3de145dc3aa11d8182f208a05397f
Date: 2025-02-25T05:06:39.916Z
Electron: 34.2.0
ElectronBuildId: 11044223
Chromium: 132.0.6834.196
Node.js: 20.18.2
V8: 13.2.152.36-electron.0
OS: Linux arm64 6.8.0-52-generic
@joshspicer
Copy link
Member Author

I suppose showing test is a bad example as that's not even a built-in here. From my spot checking I don't think any of the built-ins listed from compgen -b are being shown

Image

@meganrogge
Copy link
Contributor

meganrogge commented Feb 25, 2025

Are there any errors in the dev tools console? They're working for me

Image Image

@meganrogge meganrogge added the info-needed Issue requires more information from poster label Feb 25, 2025
@Tyriar
Copy link
Member

Tyriar commented Feb 25, 2025

Related: #241822

@Tyriar Tyriar added this to the February 2025 milestone Feb 25, 2025
@joshspicer
Copy link
Member Author

joshspicer commented Feb 25, 2025

Ok, I am seeing them now (even on fresh terminal). Nothing in devtools, but will keep an eye out for any hints.

Image

@meganrogge
Copy link
Contributor

Would be great if you can find a repro for when this doesn't work.

@Tyriar
Copy link
Member

Tyriar commented Feb 25, 2025

@meganrogge when it happens I always see the file path, so it seems like entries off PATH are taking priority over builtins, I think what we actually want to do here is always take the builtin instead.

@meganrogge
Copy link
Contributor

The logic already works to have the builtin preferred over the executable. I think this suggests there's a case when the builtins are not able to be found/ there's an error in fetching them.

@meganrogge
Copy link
Contributor

The order in which we add builtins/executables make it impossible for the executable to override the builtin. Thus why I think in this edge case, there's a problem fetching builtins.

// Order is important here, add shell globals first so they are prioritized over path commands
const commands = [...shellGlobals, ...commandsInPath.completionResources];

const labels = new Set(items.map((i) => typeof i.label === 'string' ? i.label : i.label.label));
for (const command of availableCommands) {
const commandTextLabel = typeof command.label === 'string' ? command.label : command.label.label;
if (!labels.has(commandTextLabel)) {
items.push(createCompletionItem(
terminalContext.cursorPosition,
prefix,
command,
command.detail,
command.documentation
));
labels.add(commandTextLabel);
} else {
const existingItem = items.find(i => (typeof i.label === 'string' ? i.label : i.label.label) === commandTextLabel);
if (!existingItem) {
continue;
}
const preferredItem = compareItems(existingItem, command);
if (preferredItem) {
items.splice(items.indexOf(existingItem), 1, preferredItem);
}
}

@meganrogge
Copy link
Contributor

When you see this, I expect an error like Error getting info for... in the console.

@Tyriar Tyriar modified the milestones: February 2025, March 2025 Feb 26, 2025
@meganrogge
Copy link
Contributor

Will add logs #242076 to help

Copy link

This issue has been closed automatically because it needs more information and has not had recent activity. See also our issue reporting guidelines.

Happy Coding!

@vs-code-engineering vs-code-engineering bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

3 participants