Skip to content
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

Add variants of editing/navagiting methods that take relative index argument (paragraph index, column index) #331

Merged
merged 2 commits into from
Jun 21, 2016

Conversation

JordanMartinez
Copy link
Contributor

No description provided.

selectRange(anchor, pos);
break;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems a little repetitive wrt. the method above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah... I'll fix that.

@JordanMartinez
Copy link
Contributor Author

I chose StyledDocument as the place to implement the base code for getAbsoluteCharPosition and not TwoDimensional because then that method would "pollute" its implementors with a confusing method name and parameters since it doesn't use int paragraphIndex and int columnIndex as its parameters but int major and int minor.

@TomasMikula
Copy link
Member

Looks, good, just not sure if I want to use "Char" in the method names. I try to be consistent in using the term position when pointing to a place between characters, and character index when pointing to a character.

 ┌ character 0
 | ┌ character 1
 | |   ┌ character 3
 | |   |
 v v   v

|_|_|_|_|

^ ^     ^
| |     |
| |     └ position 4
| └ position 1
└ position 0

@JordanMartinez
Copy link
Contributor Author

I agree with keeping it consistent.

At the same time, I don't know if this distinction between position and character index is even stated in the javadoc or is as clear as the above picture.
Should it be in the javadoc? Or somewhere else in the project pages (ReadMe, Wiki, etc.)?

@JordanMartinez
Copy link
Contributor Author

Well, since so many methods point to getAbsolutePosition(), whose javadoc now includes your picture, I think that takes care of the previous issue I raised.

@@ -197,4 +265,39 @@ default void replace(IndexRange range, StyledDocument<PS, S> replacement) {
*/
@Deprecated
public void positionCaret(int pos);

/**
* Returns the absolute position (e.g. the spot in-between characters) to the left of the given column in the given paragraph.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean "i.e." instead of "e.g." ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used "i.e." a lot until I saw you use "e.g." a lot. So, I thought they were the same and just used that whenever it related to you or this project. 😃 Having looked it up, I see the difference now.

… argument (paragraph, column) rather than an absolute position (current approach)

Every method that uses the `getAbsolutePosition` method includes a note about the potentially unexpected position returned if a column index argument spans outside of its corresponding paragraph's bounds.
@TomasMikula
Copy link
Member

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants