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

Feature/folder view #85

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

HWienhold
Copy link
Contributor

tried to tackle #82
And because of your great hint i m not so scared of regex-syntax/src anymore (but srsly: do not try this branch w/ regex-sytax)

Although this kind of destroys the whole grep through all files kind of advantage, but maybe thats an issue for another time.

@xfbs
Copy link
Owner

xfbs commented Nov 3, 2024

Hey, thanks for this!

I just took a look at it, the code looks good tbh.

I did a bit of local testing. Am a bit worried about one thing: when I run it (even in release build), when you select a folder that has a lot of files in it, there is a noticeable lag (maybe ~1s) before it shows the files.

For example: http://localhost:8080/syn/2.0.64/2.0.87/benches, then select src.

There is something in Yew where when you have a long list of things, you should give them unique key={...} attributes so that yew can efficiently manipulate them. Maybe in src/views/diff.rs line 309 we should change it to be <LazyDiffView key={full_path} ... />? I don't have enough time to play around with it at the moment, but that might fix it.

Another question would be: Should we display some kind of root folder, such that when you click on it, it shows all changed files? This is what GitHub does. If we do: when you click on a file, it could scroll to the file instead of only showing that file.

However, I'm also a bit worried that we now have two (different) ways to view files. One is to open a folder and scroll to the file, the other one is to click on it. Having it always be scroll-based would make it easy to skim all changes, but it means we have to rely on more hacks (like the use_visible() hook) to make it perform well.

I'll have more time soon to debug it a bit more :)

@HWienhold
Copy link
Contributor Author

HWienhold commented Nov 3, 2024

Yes, i noticed that - i just tried adding a key - i d say it "feels" a bit faster, but i could be mistaken, (but may look at it in a couple of days - maybe nead to bench it). In any case, the lag seem to be still apparent. Maybe bc of this a root folder view with all files and scrolling instead of separate views should be postponed?

Just to remind you - the carlton gif would remove the need of another view :)

@HWienhold
Copy link
Contributor Author

So i did at least a little bit testing (not so much tbh)

without any changes i get1:

grafik

but the duration varies and might get here as hig as ~1,9s.

I am still not convinced that the keys do that much, but they might a bit.

But how could it? When calling src/hir/translate.rs on its own (that would be the first file) i get:
grafik

so creating this single filediff already took almost a second here.

One optimization might be the increase the estimation2 of the placeholder, or droop em ltogether and set higher than a normal screen height.

Footnotes

  1. I used regex-syntax/src

  2. in this case here 3 files are loaded, yet only two are visible bc the 3rd is almost on screen

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.

2 participants