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

Feature: Provide mapping between all-paragraph and visible-paragraph index systems #599

Closed
JordanMartinez opened this issue Sep 30, 2017 · 5 comments

Comments

@JordanMartinez
Copy link
Contributor

JordanMartinez commented Sep 30, 2017

Given an index in area.getParagraphs(), one should be able to map it to its respective area.getVisibleParagraphs() index. Likewise, the index of the latter should be mappable to the index of the former. Thus, we should add an API like

public final Optional<Integer> allParToVisibleParIndex(int allParIndex);
public final int visibleParToAllParIndex(int visiblePar);
@grenche
Copy link

grenche commented Jun 28, 2018

I think there may be a bug in allParToVisibleParIndex(). First (a minor thing), the error message for if the index given is negative says "Visible paragraph index" when I believe it should say "All paragraph index" or something along those lines.

My real question is regarding the second if statement and the point of the method in general. Based on the name and short description, I assumed that this method returned the visible index given an index related to the entire area. If that's the case, I don't understand why it should matter if the index is within the size of the visible paragraphs. I'd imagine you'd want a check for whether or not it is visible, but I don't see why the other check is necessary.

If I'm actually just misunderstand the point of this method, is there a way of getting the visible index from the general index?

@JordanMartinez
Copy link
Contributor Author

First (a minor thing), the error message for if the index given is negative says "Visible paragraph index" when I believe it should say "All paragraph index" or something along those lines.

Yup, that's a typo.

My real question is regarding the second if statement and the point of the method in general. Based on the name and short description, I assumed that this method returned the visible index given an index related to the entire area. If that's the case, I don't understand why it should matter if the index is within the size of the visible paragraphs. I'd imagine you'd want a check for whether or not it is visible, but I don't see why the other check is necessary.

Looking at the code again, that does look like a bug. I think it's supposed to check whether the allParIndex argument is within the size of the area's total paragraphs (e.g. area.getParagraphs().size()), not the visible paragraphs.

@JordanMartinez
Copy link
Contributor Author

@grenche I made a new snapshot release with the fix.

@grenche
Copy link

grenche commented Jun 28, 2018

@JordanMartinez Great! Thank you. How can I use it?

@JordanMartinez
Copy link
Contributor Author

@grenche see the Readme's instructions on the snapshot release.

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

No branches or pull requests

2 participants