fix inconsistent EOL and improve readability #6
Conversation
Allow TextStyle on pc:Page, fix PRImA-Research-Lab#7
Changed namespace to 2019
|
The whitespace changes are difficult for us to adopt because we use the Eclipse schema editor which creates the spaces/tabs/breaks in such a way. |
|
Are you entirely sure of that? It looks to me as if different editors/rules were mixed here throughout the history of the document: There are both Unix and Windows line breaks, both space- and tab-based indentation. Since versioning is based on textual diffs, we should strive to at least get these consistent. (Also, many editing tools would not tolerate this.) BTW, I have just tried with Eclipse 4.11's XSD editor and it can import the file just fine (and also re-export without introducing new inconsistencies). But perhaps you want me to change this PR so that rebasing is used instead of merging? Or should I rather base it on the 2018 version (i.e. before b2ffba9 etc)? |
|
Okay, that is entirely possible. But I do remember changing documentation text and the editor changed it to the weird padded layout. Also I'm a few versions behind with Eclipse, but that shouldn't make a difference. |
|
I used Eclipse's clean-up features, hope that helps. converted to UNIX line breaks. |
Oh right, I forgot. I should probably have submitted it to the main repo. But I guess that is not an option any more, since:
Oh no! Now you have done some cleanup yourself, but in a different direction. Why did you ignore my effort completely? Please have at least a look at the changelog above. My changes were systematic and much more thorough (and made by hand). They improved readability and clarity for other (more text-oriented) editors as well. Now we have no common baseline anymore, and you already mixed in new features ( |
|
Sorry I made a mess. I tried to include your text changes, but might have missed some because of the many changes in the diff. I had missed your "more readability". I can work that in. Or do you have a recommendation how to fix this? |
|
PS: A complicating factor is that we use SVN as main repository, the GitHub repo is only a secondary one. Should probably move fully to GitHub |
|
PPS: The underlineStyle is only a tiny change and in a separate commit, so I can do it again if it's easier to roll back. |
Oh, I see. Then you probably merged manually, or just tried to re-do my changes? (IMHO, you would not need to move to Git exclusively, just follow certain principles/recipes when syncing both repos. Generally, the idea is to use the working directory as a means of communicating between Git and SVN – instead of push/pull –, but re-commit changesets on the other side – possibly squashing multiple changesets and supplying independent changelog, or even re-doing every single one of them including changelog.)
Sorry, not knowing what your exact tooling is, I currently have no recommendation how to proceed. Maybe I should look into the diffs again... |
|
@chris1010010 It would be great if you could roll back your cherry picking of @bertsky proposals. I think they are very helpful and thoroughly carried out. OCR-D is ready to merge them and we would be very thankful if you were too. And yes, migrating to GitHub would be wonderful. |
|
@bertsky @wrznr |
|
@chris1010010 splendid. I will. (And I will use rebasing so the merge above disappears.) |
- indent consistently (only tabs instead of mixed with spaces, proper indentation level without changing too much) - break too long lines, especially documentation - improve word wrapping in documentation - close elements on the next line if they had content - prefer collapsed closing form if possible - improve documentation of readingDirection/textLineOrder
- remove use="optional" in attributes (it is default), add a reminder comment of important defaults on top - more indentation and collapsing - make GraphemeBaseType abstract
|
@chris1010010 done. Just in time, I noticed that I also had to omit fa1b7ba ("Add new TextStyle attribute doubleUnderlined" from @tboenig), which was ahead of upstream on this fork, because you are going to supersede that by your new I opened a new PR on the origin, where it does match. I noticed your own attempt in fixing the documentation for |
No description provided.