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

Lets 'ipfs add' include top-level hidden files #2202

Merged
merged 1 commit into from
Jan 15, 2016

Conversation

hackergrrl
Copy link
Contributor

Fixes ipfs/go-ipfs/#2145. The --hidden switch (still) only affects recursive adding.

License: MIT
Signed-off-by: Stephen Whitmore noffle@ipfs.io

@jbenet
Copy link
Member

jbenet commented Jan 15, 2016

maybe hidden files should act on non-recursive adds too, unless -H?

> mkdir /tmp/foo
> cd /tmp/foo
> git init
> touch bar
> echo "bar" >.gitignore
> git add bar
The following paths are ignored by one of your .gitignore files:
foo
Use -f if you really want to add them.

nvm

return err
// Iterate over each top-level file and add individually. Otherwise the
// single files.File f is treated as a directory, affecting hidden file
// semantics.
Copy link
Member

Choose a reason for hiding this comment

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

good catch!

@jbenet
Copy link
Member

jbenet commented Jan 15, 2016

Sorry, i'm being silly. this is for hidden files, not for ignored files.

@@ -48,6 +48,17 @@ test_add_skip() {
test_cmp expected actual
'

test_expect_success "'ipfs add' includes hidden files at the top-level even without --hidden" '
Copy link
Member

Choose a reason for hiding this comment

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

maybe say 'ipfs add' includes hidden files given explicitly even without --hidden -- top-level was a bit confusing given the example below (mountdir/dotfiles/.vimrc), took me a second to realize it meant "top level of the commands file tree"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're 100% right -- that's pretty confusing. I'll steal yours. :)

@jbenet
Copy link
Member

jbenet commented Jan 15, 2016

one way to do the "explicit/direct" thing is to have the commands lib label the files that are added directly vs those added recursively, then the adder can check something like:

if f.Explicit() || adder.Hidden { ... }

@jbenet
Copy link
Member

jbenet commented Jan 15, 2016

@noffle comments above, otherwise LGTM. +1 on comments, thank you.

@hackergrrl
Copy link
Contributor Author

one way to do the "explicit/direct" thing is to have the commands lib label the files that are added directly vs those added recursively

Happy to have you sell me on it, but I really don't like the idea of the exposing the semantics of how command line processing works to the FileAdder -- I think this sort of "hidden unless an 'explicit' file" notion should live on the outside.

What might be nice is it FileAdder exposed a callback func that you could provide that acted like a filter function. Each file calls it (ideally with enough information to infer whether it's top-level/explicit or not), and so those sorts of policy decisions (filter or include) can be made from the outside without polluting the FileAdder.

@jbenet
Copy link
Member

jbenet commented Jan 15, 2016

Oh i understand now-- i was missing that explicitly added single files will always be in top level, obvious in retrospect.

@jbenet
Copy link
Member

jbenet commented Jan 15, 2016

ok LGTM! -- can you squash 01bb593 into 15554b1 (or squash the whole thing?) we have a requirement in this repo that all commits should pass the tests so that we can git-bisect easily.

@jbenet
Copy link
Member

jbenet commented Jan 15, 2016

(last thing and good to merge i think!

@whyrusleeping
Copy link
Member

cool, this LGTM

@hackergrrl
Copy link
Contributor Author

we have a requirement in this repo that all commits should pass the tests so that we can git-bisect easily.

That is an excellent requirement. Can do! 🎊

Fixes ipfs/go-ipfs/ipfs#2145. The --hidden switch (still) only affects
recursive adding.

License: MIT
Signed-off-by: Stephen Whitmore <noffle@ipfs.io>
@whyrusleeping
Copy link
Member

cool cool cool 🚢

whyrusleeping added a commit that referenced this pull request Jan 15, 2016
Lets 'ipfs add' include top-level hidden files
@whyrusleeping whyrusleeping merged commit b182e53 into ipfs:master Jan 15, 2016
RichardLitt added a commit that referenced this pull request Jan 16, 2016
Because this requirement is not listed anywhere, but was mentioned in #2202 (comment).

License: MIT
Signed-off-by: Richard Littauer <richard.littauer@gmail.com>
RichardLitt added a commit that referenced this pull request Jan 23, 2016
Because this requirement is not listed anywhere, but was mentioned in #2202 (comment).

License: MIT
Signed-off-by: Richard Littauer <richard.littauer@gmail.com>
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.

3 participants