Auto copy with safecontent

[AnomalRoil]

With this, passwords will be copied to clipboard when safecontent is set and there is only a password in the entry invoked.
This is addressing #684.

I've adapted the unit tests accordingly and I've added a unit test to check for conflict between safecontent and -c in the show action. (To avoid regressions later on, as the expected behaviour when specifying -c is to copy to clipboard, not to show the (safe) content)

[AnomalRoil]

Wow, this failed for a reason I wasn't expecting:

Error: Not equal: 
	   expected: "No safe content to display, you can force display with show -f.\nCopying password instead.\nCopied fixed/secret to clipboard. Will clear in 45 seconds."
	   actual  : "No safe content to display, you can force display with show -f.\nCopying password instead.\nWARNING: No clipboard available. Install xsel or xclip or use -p to print to console"

Should I fix it by adapting the unit test to reflect Travis lack of xsel or xclip, or should I fix it by modifying travis.yml to add xclip to its apt packages?

[dominikschulz]

Hi,
thanks a lot for this PR. This should be a useful addition.

Please set clipboard.Unsupported = true in the test case to make it pass, see https://github.com/justwatchcom/gopass/blob/master/action/clipboard_test.go#L16

[AnomalRoil]

I guess I've misunderstood something there.

What's clipboard.Unsupported = true supposed to do?
Its doc says:

Unsupported might be set true during clipboard init, to help callers decide whether or not to offer clipboard options.

The problem I'm having seems to be that I'm expecting the copy action to succeed with its standard message, but since Travis isn't featuring xclip, it fails.

A solution could be to simply add xclip in the Travis config file, when installing gnupg and git.

[dominikschulz]

Oh, my bad. I missed the fact that you added an integration test, not a unit test.
While integration tests are a good safety net, the have some limitations and are generally slower than unit tests. Why don't you just turn the integration test into a unit test? There setting clipboard.Unsupported will work because of https://github.com/justwatchcom/gopass/blob/master/action/clipboard.go#L18

Just add a new test case here: https://github.com/justwatchcom/gopass/blob/master/action/clipboard_test.go

Have a look at the other unit tests on how we mock/capture input/output. Let me know if you need help.

[dominikschulz]

Actually I think you could just change this line https://github.com/justwatchcom/gopass/pull/685/files#diff-a9331e67576623a133a012a0cfd71beeR40 to assert.Contains(t, out, "No safe content to display, you can force display with show -f.\nCopying password instead.").

This should work.

[AnomalRoil]

True. But it's kind of a workaround and it wouldn't cost me much to add it to the actual unit tests.

Is there any benefits to have a same test both as a unit test and as an integration test? (I was thinking those were all unit tests.)

[dominikschulz]

Both have their merits. Unit tests are much faster and easier to implement, but you need to do a lot of mocking so you might change the behavior without noticing.

Integration tests are usually more annoying but they test the final binary without any mocking, so they give greater confidence.

[codecov]

Codecov Report

Merging #685 into master will increase coverage by 0.08%.
The diff coverage is 66.67%.

@@            Coverage Diff             @@
##           master     #685      +/-   ##
==========================================
+ Coverage   64.38%   64.45%   +0.08%     
==========================================
  Files         150      150              
  Lines        8450     8451       +1     
==========================================
+ Hits         5440     5447       +7     
+ Misses       2344     2338       -6     
  Partials      666      666

Impacted Files	Coverage Δ
action/show.go	52.94% <66.67%> (+7.7%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57514d0...34c79c2. Read the comment docs.

[AnomalRoil]

Alright, I've workaround all the problems I had... At last!

Maybe I should rebase, since I'm reverting a lot of stuff in the end?

[dominikschulz]

Yes, please rebase onto master git rebase master and possibly squash your changes git rebase -i master.

[AnomalRoil]

Done.

I haven't squashed everything into just one commit, because those 3 commits are doing 3 different things... But if you want, I can squash everything into just one.

[dominikschulz]

No, this is fine.

[dominikschulz]

LGTM. Thanks a lot!