Skip to content

Conversation

@weilirs
Copy link
Collaborator

@weilirs weilirs commented Mar 24, 2025

 

Screen.Recording.2025-03-24.at.11.41.05.mov
@weilirs weilirs requested a review from milaiwi March 24, 2025 15:44
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR implements a scroll-to-content feature for search results and similar entries, allowing users to navigate directly to relevant content when opening files.

  • Added sophisticated content matching in FileContext.tsx with multiple fallback strategies (exact match, partial match, word-by-word search) to handle various text scenarios
  • Implemented text extraction logic in DBResultPreview.tsx and SimilarEntriesComponent.tsx to identify meaningful content snippets for scrolling
  • Added contentToScrollTo parameter propagation through context providers and components to support the scroll feature
  • Potential timing issue in FileContext.tsx with 800ms setTimeout that could be unreliable for very large documents
  • Security consideration: Content extraction uses removeMd for sanitization, but should validate input lengths and content types

💡 (5/5) You can turn off certain types of comments like style here!

6 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile

const firstFewLines = lines.slice(0, 5)
const titleLine = firstFewLines.find((line) => {
const trimmed = line.trim()
return trimmed && trimmed.length > 3 && trimmed.length < 100
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: length > 3 may miss valid single-word titles

content = (
<div className="size-full">
{similarEntries
.filter((dbResult) => dbResult)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: filter(dbResult => dbResult) won't catch empty content - should be more specific

Comment on lines +105 to 107
onSelect={(path, content) => {
openTabContent(path, undefined, false, content)
posthog.capture('open_file_from_related_notes')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: The undefined parameter passed to openTabContent could be removed since it's not being used meaningfully

Comment on lines +13 to 17
const getFileName = (path: string) => {
if (!path) return null
const parts = path.split('/')
return parts[parts.length - 1]
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: getFileName now returns null for invalid paths but this isn't handled in the UI rendering - could cause runtime errors

Suggested change
const getFileName = (path: string) => {
if (!path) return null
const parts = path.split('/')
return parts[parts.length - 1]
}
{fileName ? <span className="text-xs text-gray-400">{fileName} </span> : null} | Similarity:{' '}
Comment on lines +189 to +190
// @ts-ignore - HTMLElement does have scrollIntoView
bestMatchElement.scrollIntoView({ behavior: 'smooth', block: 'center' })
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Remove @ts-ignore and properly type HTMLElement. scrollIntoView is a standard method that shouldn't need ignoring

Suggested change
// @ts-ignore - HTMLElement does have scrollIntoView
bestMatchElement.scrollIntoView({ behavior: 'smooth', block: 'center' })
bestMatchElement.scrollIntoView({ behavior: 'smooth', block: 'center' })
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant