Differential D7848

added the "remove this bookmark" to bookmarks section
ClosedPublic
Actions

Authored by lexdem on Sep 16 2017, 2:35 PM.

Details

Reviewers
aacid
Group Reviewers
Okular
Commits
R223:a57ffdbd959c: added the "remove this bookmark" to bookmarks section
Summary

FEATURE: 357626
Remove bookmark from popup menu added

Diff Detail

Repository
R223 Okular
Lint
Lint Skipped
Unit
Unit Tests Skipped
lexdem created this revision.Sep 16 2017, 2:35 PM
Restricted Application added a project: Okular. · View Herald TranscriptSep 16 2017, 2:35 PM
lexdem updated this revision to Diff 19589.Sep 16 2017, 3:26 PM

removed typo in "rename"

aacid added a subscriber: aacid.Sep 16 2017, 4:21 PM

Please make the summary be correct according https://community.kde.org/Policies/Commit_Policy#Special_keywords_in_GIT_and_SVN_log_messages

part.cpp
2204

Why commented code?

lexdem edited the summary of this revision. (Show Details)Sep 16 2017, 5:42 PM
lexdem updated this revision to Diff 19594.Sep 16 2017, 5:45 PM

Removed commented lines

aacid added inline comments.Sep 16 2017, 6:43 PM
part.cpp
2187

Any reason you're not just calling

BookmarkManager::removeBookmark( const DocumentViewport &vp );

?

lexdem updated this revision to Diff 19595.Sep 16 2017, 6:59 PM

Shortened from searching bookmark to just remove with viewport

aacid added a comment.Sep 16 2017, 7:32 PM

Did you upload a wrong diff?

lexdem updated this revision to Diff 19597.Sep 16 2017, 7:54 PM

Fixed wrong diff

lexdem added a comment.Sep 16 2017, 7:54 PM
In D7848#146378, @aacid wrote:

Did you upload a wrong diff?

Sorry for such a mess. First time commiting

part.cpp
2187

Just checked with that. It works, but on my machine there were some "slow responding". Not sure, if it just because of lot memory used. But anyway, here goes new diff

2204

Actually, not sure, if need this lines. Okay, I'll remove, since it works without it

aacid added a comment.Sep 16 2017, 8:03 PM

I have to ask, i need your real name to give you proper copyright attribution and "Lex Dem" doesn't really look like a russian name, could you please confirm your real name?

lexdem added a comment.Sep 16 2017, 8:10 PM
In D7848#146398, @aacid wrote:

I have to ask, i need your real name to give you proper copyright attribution and "Lex Dem" doesn't really look like a russian name, could you please confirm your real name?

Alexey Demennikov. Thanks. Shortened name for faster typing :)

aacid accepted this revision.Sep 16 2017, 8:15 PM
This revision is now accepted and ready to land.Sep 16 2017, 8:15 PM
aacid closed this revision.Sep 16 2017, 8:16 PM

Revision Contents
Changeset List

PathPackages
Mpart.h (2 lines)
Mpart.cpp (23 lines)

Diff 19597

View Options

part.h

Loading...
View Options

part.cpp

Loading...