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

Improve support for submodule diffs #351

Open
phil-blain opened this issue Sep 28, 2019 · 4 comments
Open

Improve support for submodule diffs #351

phil-blain opened this issue Sep 28, 2019 · 4 comments
Labels
enhancement needs info Devs need more info to address this

Comments

@phil-blain
Copy link
Contributor

First, great project!!

I noticed that the support for diffs including submodule changes could be improved. Some examples :

  1. Let's say I have a project with a submodule ("brooklyn"), and I modify a file in the submodule :

    $ git status
    On branch add-sub
    Changes not staged for commit:
      (use "git add <file>..." to update what will be committed)
      (use "git checkout -- <file>..." to discard changes in working directory)
      (commit or discard the untracked or modified content in submodules)
    
        modified:   brooklyn (modified content)
    
    no changes added to commit (use "git add" and/or "git commit -a")

    the output of git diff, using diff-so-fancy is

    $ git diff
    ────────────────────────────────────────────────────────────────────────────────────
    modified: brooklyn
    ────────────────────────────────────────────────────────────────────────────────────
    @ brooklyn:1 @
    Subproject commit 30f3d2754a98670d57d142193ba51af3038555af
    Subproject commit 30f3d2754a98670d57d142193ba51af3038555af-dirty
    

    that's good, but it could be useful to have the header be

    modified: brooklyn (modified content)
    

    to reflect the output of git status. SImilarly, if new commits were added to the submodule, the header could indicate it : modified: brooklyn (new commits)

  2. I usually set diff.submodule log. With this setting, the brooklyn submodule looses its header with (or without) diff-so-fancy :

    $ git diff --submodule=log
    Submodule brooklyn contains modified content
    $ git -P diff --submodule=log
    Submodule brooklyn contains modified content
    
  3. Same thing with git diff --submodule=diff, the submodule itself doesn't have a header but its modified files do have one :

    Submodule brooklyn contains modified content
    ────────────────────────────────────────────────────────────────────────────────────
    modified: brooklyn/README.md
    ────────────────────────────────────────────────────────────────────────────────────
    @ brooklyn/README.md:93 @ Useful topics include:
    # diff here
    
@scottchiefbaker
Copy link
Contributor

I've never used sub-modules. If you can submit a test case that adequately recreates this issue in a reliable and repeatable manner I can investigate this new feature..

@scottchiefbaker scottchiefbaker added enhancement needs info Devs need more info to address this labels Apr 23, 2020
@abrioy
Copy link

abrioy commented May 5, 2020

I'll try to provide a test case for you using both values of diff.submodule.

I'm using a demo repo from github where the css and js folder are submodules.

git clone --recurse-submodules https://github.com/githubtraining/example-dependency
cd example-dependency

cd js
# Making some changes in a submodule
echo -e "\n\n Committed test diff in README" >> README.md
git commit -am "Update README"
echo -e "\n\n Dirty test diff in README" >> README.md

cd ..
# This diff shows the committed changes to the submodule
git config diff.submodule log
git diff 

# This one shows the full diff
git config diff.submodule diff
git diff

Attached are the patch files for both examples :

example1.patch.txt
example2.patch.txt

Thank you for this tool, A header for the submodules would be a great addition.

@phil-blain
Copy link
Contributor Author

@abrioy I've been trying https://github.com/dandavison/delta and it does add a submodule header.

@scottchiefbaker
Copy link
Contributor

I will investigate this now that I have some other stuff off my plate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs info Devs need more info to address this
Projects
None yet
Development

No branches or pull requests

3 participants