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

Show file mode changes #543

Open
tkazik opened this issue Jul 29, 2021 · 8 comments
Open

Show file mode changes #543

tkazik opened this issue Jul 29, 2021 · 8 comments
Assignees
Labels
feature request Feature request nice to have A feature request that is nice to have (lower priority)

Comments

@tkazik
Copy link

tkazik commented Jul 29, 2021

It would be great if you could show the file mode changes as well, e.g. from 755 to 644. Currently, this is not visible, i.e see the partial screen shot, where the content of the file did not change:

git_graph_file_mode

Keep up the amazing work!

@tkazik tkazik added the feature request Feature request label Jul 29, 2021
@mhutchie
Copy link
Owner

Hi @tkazik,

Thank you for raising this feature request, it would be a nice addition to have.

@mhutchie mhutchie added the nice to have A feature request that is nice to have (lower priority) label Jul 29, 2021
@KarlHeitmann
Copy link

KarlHeitmann commented Jul 31, 2021

Hi! With the arg --summary I could get the mode changes before and after with git. Here is an example on how it looks.

image

The work is in progress some refactorization is needed, and I need to make some tests. But this screenshot is for any feedback you may have. Maybe I should put a tooltip, or use another color, I don't know.

Here is another screenshot with no deletions nor additions: only modes changed.

image

@mhutchie
Copy link
Owner

mhutchie commented Aug 1, 2021

Hi @KarlHeitmann,

Thanks for experimenting with this feature.

In order for this to be implemented, it ought to be displayed as a symbol/icon next to the file, with a tooltip displaying the change that occurred. This is because it is critical to keep records in the Commit Details View File View as short as possible (as space is limited when users operate in repositories with deeply nested folders and long file names). The symbol should only be shown if the file mode has changed, to further reduce space.

How are you retrieving data with the --summary argument? (i.e. does it require another Git command to be run when the Commit Detail View is opened)

@KarlHeitmann
Copy link

Hi @mhutchie

Ok, I'll search for a icon/symbol to display next to the file with the tooltip. I've already seen how you've done that with addition/deletion

I've seen that the methods "getDiffNameStatus" and "getDiffNumStat" were pretty similar, and they done a very similar job. So I copy getDiffNumStat, and I've adapted it to send the ["--sumary"] argument to execDiff, instead of sending the ["--numstat"] argument.

I am working right now on the tests, because this feature have broken some tests on the dataSource.ts test suite.

I've just uploaded the changes to my fork, if you want to take a look. Although it ain't finished yet.

Best,

Karl

@KarlHeitmann
Copy link

PS: YES, it does needs to run another git command! Should I put that --summary argument inside the getDiffNumStat method?

@mhutchie
Copy link
Owner

mhutchie commented Aug 1, 2021

Hi @KarlHeitmann,

If this feature is to be released, it can't require an additional Git command to be executed every time the Commit Details View data is loaded. It is already bad enough that two Git commands have to be run to get the required modification and change information, so I don't want to add the additional overhead of another Git command. This is because file mode changes are very infrequently used in Git repositories, so the value of this feature to users is extremely small and definitely doesn't outweigh the overhead of running another command for every user every time they load data in the Commit Details View.

If this feature can be achieved without running another Git command, and the approach is compatible with historical Git releases, then I'd be happing to include it.

@KarlHeitmann
Copy link

Ok Michael, fair enough. I will see if I can put the feature on an existing git command.

@tkazik
Copy link
Author

tkazik commented Aug 1, 2021

Great to see some interest & activity here already!
@KarlHeitmann Nice catch with the --summary argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request nice to have A feature request that is nice to have (lower priority)
Projects
None yet
Development

No branches or pull requests

3 participants