Skip to content

Conversation

czarkoff
Copy link
Contributor

Useful for navigation between the commits.

The implementation is suboptimal. It would probably make more sense to search for children when the view is displayed, but that would require much more substantial changes as currently commit list is not available in the view.

That said, in my testing this implementation appears to perform acceptably.

Useful for navigation between the commits.
@czarkoff
Copy link
Contributor Author

FWIW to me it would make sense to change parent and children lists from List<string> to List<Commit>. That would also allow for showing tooltips with useful info in the details view.

@love-linger love-linger self-assigned this Nov 10, 2024
@love-linger
Copy link
Collaborator

To be honest, I don't really like this feature - especially the design of Children.

FWIW to me it would make sense to change parent and children lists from List to List. That would also allow for showing tooltips with useful info in the details view.

I also don't agree with this modification. Relevant, if necessary, I will provide a custom text block control that used to display Commit.SHA. Only when its content changes will it asynchronously obtain the information needed for tooltips.

@czarkoff
Copy link
Contributor Author

You don't find this feature useful in principle, or you would prefer a different implementation? An alternative would be to search for the commit's children when it is being displayed. Or maybe you have some other idea?

@czarkoff
Copy link
Contributor Author

I've reverted the previous attempt and made another one. This time children are fetched asynchronously for a specific commit while the view is being rendered. If I understood your feedback correctly, that should be more in line with your approach.

Now, it is possible that I didn't understand your feedback correctly. If that is the case, I would appreciate if you could explain what kind of implementation for this feature you would like to see. Or close the PR if you don't regard this functionality as a desirable feature.

Either way I hope I didn't waste too much of your time unnecessarily, and I am really sorry if I did.

Useful for navigation between the commits.
@love-linger
Copy link
Collaborator

Nice job. It's better now.

@love-linger love-linger merged commit 03f96cc into sourcegit-scm:develop Nov 11, 2024
13 checks passed
love-linger added a commit that referenced this pull request Nov 11, 2024
* add translations for zh_CN and zh_TW
* hide `CHILDREN` line if it is empty

Signed-off-by: leo <[email protected]>
@love-linger
Copy link
Collaborator

love-linger commented Nov 11, 2024

I'm sorry. I will revert this PR due to the performance of Commands.QueryCommitChildren. It takes more than 2 seconds on my PC (AMD 7950X) when select a commit in UnrealEngine repo.

love-linger added a commit that referenced this pull request Nov 11, 2024
* the `Commands.QueryCommitChildren` takes too much time when executes in a large repo

Signed-off-by: leo <[email protected]>
@czarkoff
Copy link
Contributor Author

I see. It was much faster with repos I tried, so thank you for the pointer at a better test. I will look into this next weekend.

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