-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Minor stylistic changes #1069
Minor stylistic changes #1069
Conversation
Improving readability and adjusting some typos.
@@ -314,13 +314,13 @@ var change = [ | |||
] | |||
``` | |||
|
|||
We have now have a Delta format that is very close to the actual Delta format. | |||
We now have have a Delta that is very close to the current standard format. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'have' twice
@@ -294,9 +294,9 @@ var change = [ | |||
] | |||
``` | |||
|
|||
Since every character is described, explicit indexes and lengths are no longer necessary. This makes out of order indexes and overlapping ranges impossible to express. | |||
Since every character is described, explicit indexes and lengths are no longer necessary. This makes indexes out of order and overlapping ranges impossible to express. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure about this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original is hard to read. Will think of a better way to phrase this and make an update. But you're absolutely right, it does seem now like it turns them disorganised.
|
||
From this, we can make the easy optimization to merge adjacent equal Operations, re-introducing length. If the last Operation is a `retain`, we can also simply drop this, since it instructs us to "do nothing to the rest of the document". | ||
Therefore, we can make the easy optimization to merge adjacent equal Operations, re-introducing _length_. And since the last Operation is a `retain` we can simply drop it, for it simply instructs to "do nothing to the rest of the document". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'And since' implies that the last Operation is always a retain
but it's not true. Not sure about the other change either. 'for it simply instructs...'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean about the since, and will change it. The other one however is implying that's the only thing being instructed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second though, this is contextualised on the example shown right before the paragraph, in which the last Operation is indeed a retain. So "Since" applies in the context given.
@@ -123,7 +123,7 @@ var content = [ | |||
|
|||
But if the answer is yes, then we violate the canonical constraint since any permutation of characters having an align attribute would represent the same content. | |||
|
|||
So we cannot just naively get rid of the newline character. We have to also either get rid of line attributes, or expand line attributes to fill all characters on the line. But what if we deleted the newline from this: | |||
So we cannot just naively get rid of the newline character. We also have to either get rid of line attributes, or expand them to fill every character on the line. But what if we removed the _newline_ from it: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'every character' is better than 'all characters'?
Added some suggestions. |
Not at all, @benbro. Quite on point with your suggestions. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! A lot of the changes make the guide much easier to digest.
@@ -16,7 +16,7 @@ If you are looking for a reference on *what* Deltas are, the [Delta documentatio | |||
|
|||
## Plain Text | |||
|
|||
Let's start at the basics with just plain text. There already is a ubiquitous format to store plain text: the string. Now if we want to build upon this and describe formatted text, such as when a range is bold, we need to add additional information. | |||
Let's start at the basics with just plain text. There already is an ubiquitous format to store plain text: the string. Now if we want to build upon this and describe formatted text, such as when a range is bold, we need to add additional information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically 'an' ubiquitous is not wrong, but 'a' is far more common especially nowadays. http://english.stackexchange.com/questions/280921/a-or-an-ubiquitous
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Today I've learned something new... the more you know, right?
@@ -27,7 +27,7 @@ var content = [ | |||
]; | |||
``` | |||
|
|||
If we want to add italics, underline, and other formats, we can add this to the main object, but it is cleaner to separate `text` from all of this so we organize formatting under one field, which we will name `attributes`. | |||
If we want to add italics, underline, and other formats; we can add this to the main object, but it is cleaner to separate `text` from all of this so we organize formatting under one field, which we will name `attributes`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"If we want to add italics, underline, and other formats" is not a complete sentence so I don't think a semicolon is appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The semicolon in this case indicates the list of elements is over.
http://writing.wisc.edu/Handbook/Semicolons.html
http://www.grammar-monster.com/lessons/semicolons_in_lists.htm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not know you could use semi-colons as higher precedent separators in lists of lists. But it doesn't seem like that applies here? There is only one list, followed by an listless independent clause. None of the sources say a semi-colon can be used just as a general list separator, but just in the special case a list is in a larger list of lists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
## Line Formatting | ||
|
||
Line formats affect the contents of the entire line, so it present an interesting challenge for our compact and canonical constraint. A seemingly reasonable way to represent center aligned text is this: | ||
Line formats affect the contents of an entire line, so they present an interesting challenge for our compact and canonical constraints. This is a seemingly reasonable way to represent centered text: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I prefer "A seemingly reasonable way to represent center aligned text is this:" is the that pronoun "this" is at the end so there's no confusion what it is referring to. When it is at the beginning, the reader may wrongly associate it with a noun in the previous sentence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, will change to something more along the lines of " see as follows".
"is this" sounds a bit too prosaic.
@@ -123,7 +123,7 @@ var content = [ | |||
|
|||
But if the answer is yes, then we violate the canonical constraint since any permutation of characters having an align attribute would represent the same content. | |||
|
|||
So we cannot just naively get rid of the newline character. We have to also either get rid of line attributes, or expand line attributes to fill all characters on the line. But what if we deleted the newline from this: | |||
So we cannot just naively get rid of the newline character. We also have to either get rid of line attributes, or expand them to fill all characters on the line. But what if we removed the _newline_ from it: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use asterisks instead of underscores as it is more consistent with other Quill docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. The reason why I've used italics was to change the tone, rather than to apply emphasis. But for the sake of consistency, I'll do as you ask.
@@ -135,7 +135,7 @@ var content = [ | |||
|
|||
It is not clear if our resulting line is aligned center or right. We could delete both or have some ordering rule to favor one over the other, but our Delta is becoming more complex and harder to work with on this path. | |||
|
|||
This problem begs for atomicity, and we find this in the newline character itself. But we have an off by one problem in that if we have n lines, we only have n-1 newline characters. | |||
This problem begs for atomicity, and we find this in the _newline_ character itself. But we have an off by one problem in that if we have _n_ lines, we only have _n-1_ newline characters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same asterisk vs underscore comment
@@ -215,7 +215,7 @@ Now that Deltas may be describing changes to a non-empty document, `{ insert: "H | |||
|
|||
#### Format | |||
|
|||
Similar to deletes, we need to specify the range of text to format, and format change. Formatting exists in the `attributes` object, so a simple solution is to provide another `attributes` object to merge with the existing `attributes` object. This merge is shallow to keep things simple. A use case that both requires a deep merge and is compelling enough to warrant the added complexity has not been found. | |||
Similar to deletes, we need to specify the range of text to format along with the format change itself. Formatting exists in the `attributes` object, so a simple solution is to provide an additional `attributes` object to merge with the existing one. This merge is shallow to keep things simple. We have not found an use case that is compelling enough to require a deep merge and warrants the added complexity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay with the wording on the first sentence. Could go both ways but I think a comma is appropriate before "along"
@@ -229,9 +229,9 @@ var delta = [{ | |||
}]; | |||
``` | |||
|
|||
The only special case is when we want to remove formatting. We will use `null` for this purpose, so `{ bold: null }` would mean remove the bold format. We could have specified any falsy value, but there may be legitimate use cases for an attribute value to be `0` or the empty string. | |||
The exceptional case is when we want to remove formatting. We will use `null` for this purpose, so `{ bold: null }` would mean remove the bold format. We could have specified any falsy value, but there may be legitimate use cases for an attribute value to be `0` or an empty string (i.e. `' '`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "special case" and "the empty string" is more idiomatic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Special case" is indeed more idiomatic, but "the" in this case sounds weird. It implies there's only "that one", some sort of specificity which is not explained or necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The empty string is definitely "a thing" in programming language theory, similar to the empty set in mathematics, but I have seem it used in formal specifications as well. However "an empty string" is not wrong nor is just "empty string". The given example of ' '
is not the/an empty string however since its length is not zero.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. you're entirely right about the example.
And there does seem to be some mentions (albeit they —to my understanding— do seem slightly contextual) of "the empty string". You are most likely right about this one too, as you are better versed at language theory than I am. Changing it.
hahaha so many commits. Want me to squash? or will you please at merge?
|
||
We now have to be careful with indexes at the application layer. As noted earlier, Deltas do not ascribe any inherent meaning to any the `attributes`'s key-value pairs, nor any embed types or values. Deltas do not know that images do not have durations, text does not have alternative texts, and videos cannot be bolded. The following is a legal Delta that might have been the result of applying other legal Deltas, by an application that was not careful of format ranges. | ||
**Note:** We now have to be careful with indexes at the application layer. As mentioned earlier, Deltas do not ascribe any inherent meaning to any the `attributes`' key-value pairs, nor any embed types or values. Deltas do not know an image does not have duration, text does not have alternative texts, and videos cannot be bolded. The following is a _legal_ Delta that might have been the result of applying other _legal_ Deltas, by an application not being careful of format ranges. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same asterisk comment
@@ -258,7 +258,7 @@ var delta = [{ | |||
|
|||
#### Pitfalls | |||
|
|||
First, we should be clear that this index must refer to the index in the document **before** any Operations are applied. Otherwise, a later Operation may delete a previous insert, unformat a previous format, etc, which would violate compactness. | |||
First, we should be clear that an index must refer to its value in the document **before** any Operations are applied. Otherwise, a later Operation may delete a previous insert, unformat a previous format, etc., which would violate compactness. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer "its position" over "its value" since it is technically more correct. I do agree on using a different word than index though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. Position does fit much better. I must have been tired at this point haha.
Changed it in order to separate the list from the rest of the phrase to keep clear the general sense of the clause, using an elliptical construction.
@@ -27,7 +27,7 @@ var content = [ | |||
]; | |||
``` | |||
|
|||
If we want to add italics, underline, and other formats, we can add this to the main object, but it is cleaner to separate `text` from all of this so we organize formatting under one field, which we will name `attributes`. | |||
We can add italics, underline, and other formats to the main object if we want to; but it is cleaner to separate `text` from all of this so we organize formatting under one field, which we will name `attributes`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to work even better. It still separates the list from the rest, and it encompasses the other clause quite nicely, methinks.
Because language theory wins.
Improving readability and adjusting some typos.
I've had soooooo much fun learning about Deltas! Thank you. Truly mindblowing.