-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
add 'title' to search result node #5628
Conversation
packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx
Outdated
Show resolved
Hide resolved
packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: kpge <[email protected]>
f308771
to
5ace801
Compare
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.
It looks great!
I'll give others a chance to review as well :)
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 check
execution OK
LGTM
@@ -573,7 +573,8 @@ export class SearchInWorkspaceResultTreeWidget extends TreeWidget { | |||
const icon = node.icon; | |||
return <div className='result'> | |||
<div className='result-head'> | |||
<div className={`result-head-info noWrapInfo noselect ${node.selected ? 'selected' : ''}`}> | |||
<div className={`result-head-info noWrapInfo noselect ${node.selected ? 'selected' : ''}`} | |||
title={new URI(node.fileUri).path.toString()}> |
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.
How about labelProvider.getLongName(new URI(node.fileUri))
?
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.
labelProvider.getLongName(new URI(node.fileUri))
returns relative path, but when the workspace is multi root workspace, we should return absolute path
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.
ok for now
Generally, we should use getLongName
and adjust its implementation properly depending on the current setup in order to have consistent way of representing paths everywhere.
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.
OK, I will keep this advice in mind and make sure they have consistent way of representing paths.
@kpge in the future you can write that your PR fixes {PR #} so it's corresponding issue is automatically closed :) https://github.blog/2013-05-14-closing-issues-via-pull-requests/ |
add feature:#5627
get file path when hover on the file node in the search result , get search result line when hover on the result, no need to click it and preview.
demo:
Signed-off-by: kpge [email protected]