Add the reviewed RTL-binding changes by Fahad#249
Add the reviewed RTL-binding changes by Fahad#249aoloe wants to merge 3 commits intoHOST-Oman:rtlBindingfrom
Conversation
|
Hi @aoloe , unfortunately I couldn't compile it at leat on my linux machine. Could you please have a look? |
|
You're right, when resolving the merge conflicts I've overseen some parts that were not conflicting but were merged incorrectly. Now it compiles and should match the patch I've uploaded to the Scribus bug tracker. I'm also adding inline the questions I've written in the ticket. |
| maxXPos = qMax(maxXPos, page->xOffset()+page->width()+m_docPrefsData.displayPrefs.scratch.right()); | ||
| maxYPos = qMax(maxYPos, page->yOffset()+page->height()+m_docPrefsData.displayPrefs.scratch.bottom()); | ||
| } | ||
| if (rtlBinding == 1) |
There was a problem hiding this comment.
What was the reason for adding this?
In my tests, it seems to work correctly without.
There was a problem hiding this comment.
Actually, I don't remember but I am sure it was for a case to enforce invalid stat on the doc so it can redraw again. I couldn't reproduce the case. So we can assume the removing it is fine for now .
|
the naming BindingDirection var to docBindingDirection just for readability since it is related to scribus doc not the Scribus itself. |
Fahad-Alsaidi
left a comment
Fahad-Alsaidi
left a comment
There was a problem hiding this comment.
Great @aoloe I am learning new things from you. Thanks.
|
|
||
| ScribusDoc* doFileNew(double width, double height, double topMargin, double leftMargin, double rightMargin, double bottomMargin, double columnDistance, double columnCount, bool autoTextFrames, int pageArrangement, int unitIndex, int firstPageLocation, int orientation, int firstPageNumber, const QString& defaultPageSize, bool requiresGUI, int pageCount = 1, bool showView = true, int marginPreset = 0, int | ||
| docBindingDircetion = 0); | ||
| ScribusDoc* newDoc(double width, double height, double topMargin, double leftMargin, double rightMargin, double bottomMargin, double columnDistance, double columnCount, bool autoTextFrames, int pageArrangement, int unitIndex, int firstPageLocation, int orientation, int firstPageNumber, const QString& defaultPageSize, bool requiresGUI, int pageCount = 1, bool showView = true, int marginPreset = 0); |
There was a problem hiding this comment.
is it safe to remove newDoc function? not related to RTL binding
| if (rtlBinding) | ||
| { | ||
| counter = counter == 1 ? 0 : 1; | ||
| currentXPos += m_docPrefsData.docSetupPrefs.pageWidth |
There was a problem hiding this comment.
this makes things difficult to read but shorter 👍
|
Just a warning: I'm not 100% sure that this PR is the same as the patch in the bug tracker. I've created the PR to make it easier for you to understand which changes I've made. |
it will be great, if we can work on one repo and we finish fixing all bugs submit one patch in the bug tracker. However, do what you like... Thanks for the reviewing my patch and adding new features. |
|
It's ok for me to work on a common repo, but I would not start from the branch you have created. I would now start from the current master and apply Martin's latest patch to it. |
|
@aoloe ok i will do it but i will make the branch (rtlBindingReview?) in scribus repo so you will have full access to it directly. Is it fine with you? or you prefer in scribus host repo? |
|
The Scribus repo on Github is just a mirror. I have now created a new repo "scribus-dev" in the Scribusproject on Github:
Is it fine this way? |
|
I've now added Martin's changes and created a draft documentation for using the scribus-dev repository: https://github.com/aoloe/scribus-manual-development/blob/master/content/github-collaboration-en.md |
2 participants
A pull request with the changes I'm proposing to your patch.
The pull request is mainly for you to understand what changes I have made.
My "anti-patch" is in
https://bugs.scribus.net/view.php?id=14544#c53304