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

feat: show result path & refactor result component #38

Merged
merged 1 commit into from
Jan 19, 2023

Conversation

ParthJadhav
Copy link
Owner

closes: #33

@ParthJadhav
Copy link
Owner Author

Hey @benediktms , This PR refactors the results component, Separates the logic & adds path on right side of full drive search.

Screenshot 2023-01-14 at 12 48 13 AM

@benediktms
Copy link
Contributor

@ParthJadhav I think it looks good, some suggestions I have:

  • For files that are in a VCS directory like git, perhaps we should display the root directory, since right now it only display the path for 2 levels.
  • As a followup we could also truncate from some sort of meaningfull directory to give the user and understanding where generally the file is, for example rather than: fileParentDir/fileDir/fileName we could display root/.../fileParentDir/fileDir/fileName where root could be the VCS root

What do you think?

@ParthJadhav
Copy link
Owner Author

I actually didn't understand what VCS mean 😅

@benediktms
Copy link
Contributor

Sorry I meant version control repos like git. But I thought maybe it would be a good to be version control agnostic. But probably that can be added if and when it's needed. We could just start with git.

@ParthJadhav
Copy link
Owner Author

I'll work on it tomorrow 🤝

@ParthJadhav
Copy link
Owner Author

  • As a followup we could also truncate from some sort of meaningfull directory to give the user and understanding where generally the file is, for example rather than: fileParentDir/fileDir/fileName we could display root/.../fileParentDir/fileDir/fileName where root could be the VCS root

I still quite didn't understand... Can u give me an example?

@benediktms
Copy link
Contributor

currently only 2 parent folders are displayed: fileParentDir/fileDir/fileName. This is better but for files that are nested deeper, e.g. app/pages/page1/components/index.ts This pattern might have lots of folder named components and lots of files named index.ts. In cases like this there are 2 solutions:

  1. Always display the absoute path from the home directory
  2. Display the absolute path from the git repository root.

Both options are fine but probably option 2 is better. In cases where the total path name is very long, we should truncate the final output to not clutter the UI. E.g.:

some-project/app/pages/feature/componets/edit/state.ts

could become

some-project/.../components/edit/state.ts

otherwise depending on how deeply nested the file is, the full file path might not fit in the results view because it is too wide.

@ParthJadhav
Copy link
Owner Author

Gotcha, I'll create a different Issue for that...

@ParthJadhav
Copy link
Owner Author

Will merge this @benediktms

@ParthJadhav ParthJadhav merged commit 8d1352d into master Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

display full path for file search
2 participants