add javadoc to Cursor#add and Cursor#addEol

[nx-eric-jarosch]

The current behaviour of add is not intuitive and warrants documentation.

[quintesse]

I understand why you might describe it this way but I prefer not to mention any "pushing". Perhaps change to:

Inserts a token at the current position which causes the cursor to advance by one (the cursor state remains as it was before, if it was at the end it will still be at the end. If it pointed to a token it will still point to that same token)

[nx-eric-jarosch]

I've had a look at the javadoc for java.util.List#add(int, T):

[...] Shifts the element currently at that position (if any) and any subsequent elements to the right (adds one to their indices).

How about that something like that?

[nx-eric-jarosch]

Hi @quintesse,
I have adjusted the phrasing to match java.util.List#add(int, T) and also included your suggestion about the cursor state rermaining unchanged.

(Note: I do think it's important to mention that the current token is moved/shifted/pushed, because that was the primary point of confusion I had with this method initially)

[quintesse]

I'll accept, even though tokens don't actually have indices, so they won't actually increment. But I understandf what you mean and given a technically exact message will probably only make it longer without making it clearer.

Again thanks!

[quintesse]

Ah yes, that part of the code has changed slightly. Would you mind updating your PR so I can merge it?

[nx-eric-jarosch]

Done :)

---

I'll accept, even though tokens don't actually have indices, so they won't actually increment.

I don't understand. To me it seems they clearly do?

public class Cursor {
    private final List<PropertiesParser.Token> tokens; // <-- associates an index for each token

    private void addToken(int index, PropertiesParser.Token token) {
        if (hasToken(index)) {
            tokens.add(index, token); // <-- increments the index of shifted tokens

[quintesse]

public class Cursor

that's exactly the key here, it's Cursor that adds the idea of an index to a Token. It's like saying that letters, on the screen or in a book, have position. I'd say it's the other way around, the screen or the book is a space that can contain letters, so a position within that space can contain a letter or not. So both Properties and Cursor assign an index to Token, but if you look inside the Token class there's nothing there that deals with indices, it doesn't know anything about that (just as letters on a screen don't know at which position they exist).

But like I said, that distinction becomes very technical and explaining it in a way that is technically correct would probably mean having to make the text much longer and very likely not any clearer. (There is a reason why technical and legal texts are often long and boring 😄 )

[quintesse]

Merged, thanks!

[nx-eric-jarosch]

tl;dr: I'll agree to disagree

---

It's like saying that letters, on the screen or in a book, have position.

I suspect this is a cultural/linguistical disagreement rather than a technical one.
In my language, such claim would be entirely ordinary.

Importantly "has" != "contains"

I would argue List.add(int, E) sets this precedent quite clearly.

adds one to their (the elements') indices

Element instances do not contain their index.
But Element instances do have zero or more indices by (indirect) extension simply by being stored in the List. (or by not being stored in the list, in the case of an element having zero indices)

To mirror your wording: if you look inside the Class definitions for any element instance stored in a List, you won't find the index that the List.add documentation refers to, either.