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

for submodule's folder,if stage with /,stage all files in it in windows #681

Merged
merged 2 commits into from
Apr 20, 2017

Conversation

xieofxie
Copy link
Contributor

After adding a submodule, its folder will be shown in untracked. However, git add folder/ will add all files in folder in windows and git add folder will only add the submodule's folder. Or one can do this check in status.py, but I think in gui, adding a folder will never happen because it always shows untracked or modified files one by one

Copy link
Member

@davvid davvid left a comment

Choose a reason for hiding this comment

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

I think in general conforming paths from foo/ to foo in the staging code path seems correct, thanks for this fix. If you'd like to create a new commit with the updates I'll merge this right away.

From the sound of the comment above, it sounds like there's a bug in that "git add foo/" will treat a submodule folder as untracked files and add them to the parent, which seems like a bug in Git itself, and by filtering what we do by only calling "git add foo" we prevent that from happening. Is that correct?

If that is case, this is a good workaround to do on our side so that we are portable to old and newer versions where this bug might have been fixed.

Do you have a small recipe of git bash commands that can demonstrate the problem on windows?

It sounds like something like..

git submodule add repo
git add repo/

Does the wrong thing by staging the repo/* files into the parent repo while doing just git add repo has the desired effect of staging the submodule repo?

Am I understanding this correctly? If so, this might be a bug we can also report to the Git for Windows project itself. Thanks for your help

@@ -467,6 +467,8 @@ def stage_paths(self, paths):

for path in set(paths):
if core.exists(path):
if path[-1:] == '/':
path = path[:-1]
Copy link
Member

Choose a reason for hiding this comment

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

A slightly more idiomatic python way to write this might be,

if path.endswith('/'):
    path = path.rstrip('/')

They're both just about equivalent, except the rstrip() will consume multiple /, but that doesn't matter since we never see those in practice.

@@ -467,6 +467,8 @@ def stage_paths(self, paths):

Copy link
Member

Choose a reason for hiding this comment

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

When creating commits please sign off your work, per https://github.com/git-cola/git-cola/blob/master/CONTRIBUTING.md#sign-your-work

In the commit message editor we can use the Ctrl+i hotkey to add the Signed-off-by line. You may need to adjust your config to include your full name.

Copy link
Member

Choose a reason for hiding this comment

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

I forgot to mention that it would be good to include some of the details that were mentioned in the pull request comment in the commit message's Extended description since the actual commit didn't include any of these details.

I tend to merge PRs via cola or git itself rather than github, so it's better to include those details in the commits rather than in the github comments.

Copy link
Contributor Author

@xieofxie xieofxie Apr 19, 2017

Choose a reason for hiding this comment

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

Thanks for your advice. I add sign-off & an example describing what I found in windows

xhl added 2 commits April 19, 2017 23:21
Test on Git-2.12.2.2-64-bit (Windows 10 x64)
for untracked files like:
folder/file
folder-submodule/file
git add folder & git add folder/ both add folder/file
git add folder-submodule only adds folder-submodule
git add folder-submodule/ adds folder-submodule/file
one can also commit both the folder-submodule & folder-submodule/file
if one checks out the project above, git submodule will not show
folder-submodule but it is still in .gitsubmodule file
maybe it is a broken feature allowing one to override files in
submodule?
Signed-off-by: xhl <xieofxie@gmail.com>
davvid added a commit to davvid/git-cola that referenced this pull request Apr 20, 2017
* xieofxie/stage-submodule:
  use a more idiomatic python way to strip / in path
  for submodule's folder,if stage with /,stage all files in it in windows

Signed-off-by: David Aguilar <davvid@gmail.com>
@davvid davvid merged commit 1f16ca6 into git-cola:master Apr 20, 2017
@xieofxie xieofxie deleted the stage-submodule branch May 27, 2017 09:18
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.

None yet

2 participants