Bug 353560 - Shift-draw straight annotation line to make vertical/horizontal
Summary: Shift-draw straight annotation line to make vertical/horizontal
Status: RESOLVED FIXED
Alias: None
Product: okular
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Ubuntu Linux
: NOR wishlist
Target Milestone: ---
Assignee: Okular developers
URL:
Keywords:
: 155420 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-10-05 11:03 UTC by kglotzba
Modified: 2020-07-17 16:32 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 1.11
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description kglotzba 2015-10-05 11:03:36 UTC
When annotating a section of a document that needs attention or explains an important concept, rather than highlighting I use the line tool to make a line left of the paragraph(s). It would be nice to make it easier to create a neat-looking document by being able to make a fully vertical or horizontal line, by means of holding the shift button after starting to draw the line (much like Photoshop functions). Possibly extra 45 degrees steps would be nice.

Reproducible: Always


Actual Results:  
Holding shift now doesn't have any function.
Comment 1 Laura David Hurka 2020-06-06 14:02:25 UTC
*** Bug 155420 has been marked as a duplicate of this bug. ***
Comment 2 Bug Janitor Service 2020-07-14 21:53:48 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/okular/-/merge_requests/216
Comment 3 Laura David Hurka 2020-07-16 22:32:47 UTC
Git commit 34708565caeb5d5fe9b8c14b9cd0af1a8ab8d5fe by David Hurka.
Committed on 16/07/2020 at 22:32.
Pushed by davidhurka into branch 'release/20.08'.

Add Constrain Angle action for annotation tools, alternative to pressing Shift

This adds a KToggleAction which sets annotation tools to constrain angle mode.
It provides an alternative user interface to the Shift button, which is used to constrain angles since MR !210.
The action and the Shift button are XOR-ed, i. e. if constrain angle mode is activated, pressing Shift temporarily disables it.

The action state is remembered accross sessions, for consistency with most other actions. It should be difficult to check this action without knowing of its existence, since it is not in any toolbar or menu, just in the action collection.

The meaning of AnnotatorEngine::Modifiers was generalized a bit, moving the responsibility about whether to constrain angles back to PageViewAnnotator, because AnnotatorEngine does not know about the action.
FIXED-IN: 1.11

M  +4    -7    doc/index.docbook
M  +23   -2    ui/annotationactionhandler.cpp
M  +2    -6    ui/annotationtools.cpp
M  +4    -3    ui/annotationtools.h
M  +23   -6    ui/pageviewannotator.cpp
M  +11   -0    ui/pageviewannotator.h
M  +1    -1    ui/presentationwidget.cpp

https://invent.kde.org/graphics/okular/commit/34708565caeb5d5fe9b8c14b9cd0af1a8ab8d5fe
Comment 4 Laura David Hurka 2020-07-16 23:04:49 UTC
(The actual code implementing this feature was submitted by Luca Citi. Thanks!)
Comment 5 Luigi Toscano 2020-07-16 23:18:48 UTC
(In reply to David Hurka from comment #4)
> (The actual code implementing this feature was submitted by Luca Citi.
> Thanks!)

Can you please revert and use the authorship information in git to properly attribute the commits? (also for other similar fixes).
Comment 6 Laura David Hurka 2020-07-17 14:56:29 UTC
> use the authorship information in git

I can revert that, but how should I include the authorship information? I could add these information in the commit message, but GitLab will always set me as the “author”.

Do you mean a revert commit and a new commit, or removing the questioned comments and re-commit them, overriding history?
Comment 7 Laura David Hurka 2020-07-17 15:00:06 UTC
> a revert commit and a new commit

As alternative, to keep the mess more compact, I could append a single commit that informs about the authors of the last commits.
Comment 8 Luigi Toscano 2020-07-17 15:04:43 UTC
(In reply to David Hurka from comment #6)
> > use the authorship information in git
> 
> I can revert that, but how should I include the authorship information? I
> could add these information in the commit message, but GitLab will always
> set me as the “author”.
> 
> Do you mean a revert commit and a new commit, or removing the questioned
> comments and re-commit them, overriding history?

author and committer are different. Are you sure you can't edit the author and push a commit?
git commit --amend --author="Foo Bar <foobar@example.com>"

 This is not about gitlab, but git, just don't use the squash feature in gitlab (which we shouldn't use, we should just push clean commits).
Comment 9 Laura David Hurka 2020-07-17 15:18:50 UTC
This page tells me that we use the squash feature in GitLab.

https://community.kde.org/Infrastructure/GitLab#Create_the_Merge_Request

Do you say I should not use the Merge button in GitLab, but push merge requests from the command line? The “Check out branch” button in GitLab indicates that this is possible, so GitLab realizes when I push an MR from the command line?

But then I actually wonder why we left Phabricator...

So what should I do now?
* git revert, git commit --author, git push (i. e. append the same patches in new commits)
* git checkout -b release/20.08, git commit --author, git push --force (i. e. override history)
* append an empty commit with author information
*
Comment 10 Albert Astals Cid 2020-07-17 15:29:42 UTC
The problem here is you squashed someone else comments in the web UI but it was in your clone, so gitlab takes you as the author, because which author is it going to chose? (imagine the squashed commits had 4 different authors)

If you would have squashed in his clone it would have been fine.

In this particular scenario you should have just squashed locally, and then you would not have needed squashing on the web UI, the actual commit author would have been preserved.

As for solutions here, it's hard, since you can't really force push to non work branches on the main repo, you need to involve sysadmin for that.

If Luigi is really interested in having proper history i guess what you need to do is prepare a clean branch in your clone and ask sysadmin to fix it, but Lugigi is it really that important?
Comment 11 Luigi Toscano 2020-07-17 15:34:50 UTC
No need to reset the branch: if you push a revert and then the original change again with the correct author, the git commands will find the last author. So just 
* git revert, git commit --author, git push (i. e. append the same patches in new commits)
Comment 12 Laura David Hurka 2020-07-17 15:42:08 UTC
Thanks, I think now I understand what is going on. I *created* the new MR in GitLab, so GitLab considered me being the author, although the commits *in* the MR are from Rene. The correct behavior would have been to locally check out Rene’s branch, squash merge it including author information to release/20.08, and then non-force push that merge commit to invent.kde.org:release/20.08. Correct?

I just looked into git log. Actually the author informations for the two patches fixing this bug are correct, only the jaggy lines fix has wrong author information now.

That fix is HEAD of release/20.08 now, so removing and re-commiting it should be comparatively easy.

--Edit--

> No need to reset the branch: if [...]

Ok, will do that.
Comment 13 Albert Astals Cid 2020-07-17 15:52:56 UTC
******
The correct behavior would have been to locally check out Rene’s branch, squash merge it including author information to release/20.08, and then non-force push that merge commit to invent.kde.org:release/20.08. Correct?
******

That's one way, but the way i think would have made more sense is

locally check out Rene’s branch, squash merge it including author information to release/20.08, and then push it to a branch in your clone, let CI run and then just merge it from the web ui.
Comment 14 Laura David Hurka 2020-07-17 16:01:05 UTC
Bug 410723:
> Git commit ab7ae92a891adcb5c43435e75ea0e9ce8c078d81 by David Hurka, on behalf of René van Paassen.

Ok, should be fine now.

So a GitLab “merge”-merge merges the actual commits, as they were submitted as part of the MR. This is especially the case when there is only one commit, so GitLab won’t squash. A GitLab squash-merge makes a new commit, and uses the MR author as author. I think I got it.
Comment 15 Luigi Toscano 2020-07-17 16:32:04 UTC
Thanks!