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

Fix & make clearClipboard more robust #359

Merged
merged 5 commits into from
Mar 14, 2018

Conversation

lukedirtwalker
Copy link
Contributor

The current version had 2 bugs:

  1. The clippedText was never cleared, that isn't that much of a problem
    since it is overriden anyways
  2. The condition to check if clearing should happen was always true
    since it compared the clippedText member variable with itself. The
    clippedText variables that are create above the condition are out of
    scope.

I fixed these bugs and clear both clipboard types if they contain the
given text, ignoring the setting, which should make it more robust.

@annejan annejan self-requested a review March 12, 2018 15:10
@annejan annejan added the bug label Mar 12, 2018
@codecov
Copy link

codecov bot commented Mar 12, 2018

Codecov Report

Merging #359 into master will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #359      +/-   ##
=========================================
+ Coverage    2.92%   2.92%   +<.01%     
=========================================
  Files          36      36              
  Lines        2699    2698       -1     
  Branches      360     361       +1     
=========================================
  Hits           79      79              
+ Misses       2619    2618       -1     
  Partials        1       1
Impacted Files Coverage Δ
src/qtpasssettings.cpp 2.94% <ø> (+0.01%) ⬆️
src/qtpasssettings.h 0% <ø> (ø) ⬆️
src/mainwindow.h 0% <ø> (ø) ⬆️
src/util.cpp 6.89% <ø> (+0.33%) ⬆️
src/passworddialog.cpp 0% <0%> (ø) ⬆️
src/mainwindow.cpp 0% <0%> (ø) ⬆️

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 baf5ea8...1f7d97c. Read the comment docs.

@coveralls
Copy link

coveralls commented Mar 12, 2018

Coverage Status

Coverage decreased (-0.002%) to 1.774% when pulling 1f7d97c on lukedirtwalker:fixClipboardClear into baf5ea8 on IJHack:master.

The current version had 2 bugs:
1) The clippedText was never cleared, that isn't that much of a problem
since it is overriden anyways
2) The condition to check if clearing should happen was always true
since it compared the clippedText member variable with itself. The
clippedText variables that are create above the condition are out of
scope.

I fixed these bugs and clear both clipboard types if they contain the
given text, ignoring the setting, which should make it more robust.
@lukedirtwalker
Copy link
Contributor Author

I'll rebase to fix the conflicts

@lukedirtwalker
Copy link
Contributor Author

I have some more changes I will want to commit. Should I just create a new PR? Or append here?

@annejan
Copy link
Member

annejan commented Mar 13, 2018

Appending here is fine by me @lukedirtwalker

@lukedirtwalker
Copy link
Contributor Author

@annejan added some more commits, they don't really have to do with the initial fix. I think it is best to review commit by commit.

@annejan
Copy link
Member

annejan commented Mar 13, 2018

I will later tonight . .

@annejan annejan merged commit c642ff1 into IJHack:master Mar 14, 2018
@lukedirtwalker lukedirtwalker deleted the fixClipboardClear branch March 14, 2018 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants