-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Improve the performance of tree listing #570
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
Conversation
@@ -40,12 +41,33 @@ func renderDirectory(ctx *context.Context, treeLink string) { | |||
return | |||
} | |||
|
|||
query := ctx.Req.URL.Query() | |||
|
|||
var start, count int = 0, 100 |
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.
I prefer the count could be configured and If we ignored some files we have to give a hint on the UI.
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.
That would be better. Let me figure out how to do it properly.
|
||
var start, count int = 0, 100 | ||
|
||
if _start, ok := query["start"]; ok && len(_start) == 1 { |
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.
ctx. QueryInt ("start")
is more convenient.
} | ||
} | ||
|
||
if _count, ok := query["count"]; ok && len(_count) == 1 { |
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.
ctx.QueryInt ("count")
entries, err := tree.ListEntries() | ||
if err != nil { | ||
ctx.Handle(500, "ListEntries", err) | ||
return | ||
} | ||
entries.Sort() | ||
|
||
entries = entries[start : start+count] |
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.
lost sort?
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.
According to ftp://www.kernel.org/pub/software/scm/git/docs/git-mktree.html, that has been done by git itself when creating the tree object. And my trivial testing seems to confirm it.
When count is not set it shows no commits. When coun/startt > commits Gitea panics |
Please mark this as work-in-progress, I will finish this PR later (adding front-end controls as @lunny suggested). @Bwko Thanks for pointing it out. Edit: Or perhaps I can set the default Edit2: The latest commit should fix the bug found by @Bwko |
d3a80bd
to
1fe0d48
Compare
@typeless Good work 👍 Only the sorting is alphabetically and not folder first. |
The sorting done by git-mktree is alphabetical but it doesn't take directories into consideration.
@Bwko My bad 😆 , didn't notice the nuances. |
@@ -12,6 +12,9 @@ import ( | |||
"path" | |||
"strings" | |||
|
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.
Remove redundant empty line.
count := ctx.QueryInt("count") | ||
|
||
// Check invalid values | ||
if (start == 0 && count == 0) || start+count < start || start+count > len(entries) { |
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.
Could we replace start+count < start
with count < 0
?
@typeless Is this PR ready to review? |
@lunny The UI part is still missing if that is desired to be included in this PR. |
@typeless UI would be required yes, otherwise there may be issue in the future :( Would be nice to have it fetch and render the next set of files via API, like GitHub has |
You could also just include the |
@typeless Is this completed? |
@lunny I don't have a good idea about how the UI design should be done, before starting to get familiar with Semantic UI. Please bear with me as my spare timeslices for OSS projects are very limited recently. If someone is willing to help implement the web UI (I am really not good at it), that would be appreciated. |
OK. Please give the permissions to let maintainers to push to your branch, it's in this PR right sidebar. And I will move this to v1.2. |
@lunny Thanks. I have the "allow edits from maintainers" checked. |
@bkcsoft any update? |
@lunny I've been swamped with work unfortunately 🙁 |
Add two query strings
start
andcount
for the home page of repository. If the values are omitted or invalid they are set to 0 and the total number of rows respectively.For #502