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

Allow following file renames in commit history activity #1443

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maniac103
Copy link
Collaborator

Fixes #1419

@maniac103
Copy link
Collaborator Author

@Fs00 Please review

public boolean onOptionsItemSelected(MenuItem item) {
if (item.getItemId() == R.id.follow_renames) {
mFollowRenames = !mFollowRenames;
item.setChecked(mFollowRenames);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Question: store the setting persistently (similarly to 'wrap lines' in file viewer) or keep default as 'off'?

@Fs00
Copy link
Contributor

Fs00 commented Feb 15, 2025

This is an interesting feature, although I have a few doubts:

  • why would we need an option to enable/disable it? I think it's very easy to miss the existence of the feature if it's disabled by default, and I don't see why a user would want to deliberately ignore older history entries
  • I'm not really knowledgeable about RxJava, but what happens if the user rotates the screen while entries belonging to an "older filename" are being displayed? As far as I can see, the RenameFollowData will be lost upon fragment recreation.

Have you considered implementing this functionality with an action at the end of the list that opens the history of the previous filename (if there is one), similarly to what the GitHub web UI does? I think it could spare us dealing with paged lists and their known issues.

gh_web_file_rename

@maniac103
Copy link
Collaborator Author

why would we need an option to enable/disable it? I think it's very easy to miss the existence of the feature if it's disabled by default, and I don't see why a user would want to deliberately ignore older history entries

It's not the default behavior for git log either, but needs to be explicitly enabled (git log --follow) there as well.

I'm not really knowledgeable about RxJava, but what happens if the user rotates the screen while entries belonging to an "older filename" are being displayed? As far as I can see, the RenameFollowData will be lost upon fragment recreation.

That's probably right, need to check this. Could likely put it into the saved state.

Have you considered implementing this functionality with an action at the end of the list that opens the history of the previous filename (if there is one), similarly to what the GitHub web UI does?

No, but I consider the current solution more suited because

  • it's in line with git log
  • the GH UI solution will get 'interesting' with both the fragment and adapter setup (we'd need to insert a dummy item into the list, but how to do that with the paging setup?)

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.

Better file renaming commit history
2 participants