Mapped list with index

[Symeon94]

Note that some of this (the update of gradle) is on another PR #82 .

The purpose of this is based on issue seen in RichTextFX where we need to get the index of the line when a paragraph is changed. For that I created a mapped list which uses a mapping function with the index.

This change should be relatively safe as it is a new type of mapped list next to the existing one (and I abstracted the existing code to avoid duplication).

[Symeon94]

This is based on following issue: FXMisc/RichTextFX#1273

[TomasMikula]

I think it would be simpler to just change MappedList's mapper, which is private, to always be a BiFunction taking an additional index. Then have two constructors, one taking a Function (the current one) and the other one taking a BiFunction. The first one would just adapt the Function to a BiFunction which ignores the other argument.

[Symeon94]

That's a very good comment, I should have thought about it (sometimes one can do some over-the-top solution ;-) ).
I'm fixing and pushing the change

[TomasMikula]

This doesn't look right.

It seems that you need a BiFunction version of Lists.mappedView, and then

Lists.mappedView(mod.getRemoved(), (i, elem) -> mapper.apply(mod.getFrom() + i, elem));

[Symeon94]

Thanks for taking the time to review.
I actually have tried that before you commented, but the problem is that the index provided by the mappedView is not the index of the removed item, it is the index in a new list.

    public static <E, F> List<F> mappedView(
            List<? extends E> source,
            BiFunction<Integer, ? super E, ? extends F> f) {
        return new AbstractList<F>() {

            @Override
            public F get(int index) {
                return f.apply(index, source.get(index));
            }

            @Override
            public int size() {
                return source.size();
            }
        };
    }

If I have removed items at index 1 and 2, I want the apply to use the index 1 and 2. I guess the i in your comment will be 0 and 1 in that case, which would result in index 1 and 3 ? (mod.getFrom() + i). Or said otherwise, we are mixing two differents unrelated types of index if we do that sum.
So, I'm wondering about the correctness of that change. Maybe you have a scenario in mind where it would fail, if you describe it I can add it to the UT and make sure to fix it.

[TomasMikula]

Yes, i will range from 0 to 1. But then mod.getFrom() will always return 1. So you get 1 and 2 as a result, which is as expected.

Anyway, I haven't run it. A scenario where this should fail is if you remove a range of at least 2 elements.

[Symeon94]

I'm a bit puzzled by the logic you are laying forth. I have actually already tested that case if you look at UT testIndexedList which test the removal of index 1 and 2 (where you can see <index>-<len> below).

        // Remove an entry to the list and check changes (note that 3-7 becomes 2-7)
        strings.remove(2);
        assertLinesMatch(Stream.of("0-1", "1-4", "2-7"), lengths.stream());
        assertEquals(Arrays.asList("1-4", "3-7"), added);
        assertEquals(Arrays.asList("1-2", "2-3"), removed);

It is my understanding that mod.getFrom() returns the original index (as has been tested with the UT). I'm not sure where you are getting that mod.getFrom() will always return 1?

If I'm mistaken somewhere, please let me know (as you created the library, you definitly have a better understanding of the overall picture than I do).

[TomasMikula]

I could easily be mistaken, as I wrote the library 10+ years ago :)

However, in the code you quoted, you only remove a single element at a time (strings.remove(2)). Try removing a range (e.g. strings.remove(2, 4)).

For the same mod, mod.getFrom() always returns the same number. That number is 1 if the change begins at index 1, as in your example.

[Symeon94]

There is no need to parse all the lines, I have already the code to identify the style change in only the visible portion of the code. The problem is that RichTextFX re-render and then applies the style after re-rendering, creating a visible delay between the two (which should be fixed with the index). I'll try to investigate when I have time on your proposal to play directly with the document prior to re-rendering and see if that can help.

(I'm not sure why you are proposing to parse the whole file each time, that is not something I'll do, as we reevaluate the style each time a user press a key, I cannot have a lag of 200ms on each key press).

[TomasMikula]

I'm not sure why you are proposing to parse the whole file each time

I'm not 😉
However, I did think you maintained an up-to-date version of all the styles.

Anyway, it seems to me that you are maintaining the styles somewhere on the side, while I'm saying the style should come through EditableStyledDocument.

If you do not want to maintain styles of the whole document, you should still be able to compute them lazily in EditableStyledDocument#getParagraph(int) (e.g. for scrolling) plus eagerly for each edit.

[Symeon94]

For more precisions, I'm only keeping track of Python open/close group comments in my file. The rest of Python syntax can be computed based on the line content, which I do and only assess on changing the visible lines or on modifying a line. So each time a user changes a line, I use the information I saved about group comments to tell if the line is commented, and if not (or for the part that isn't) I recompute the style.

The styles are in the document, but it is hard for me to maintain that multiline comment based uniquely on the style class in the EditableStyledDocument<Collection<String>, String, Collection<String>>.

And, in any case, I don't think the style calculation is the right conversation here. The only problem is with a delay in the display.

Note that I'm not having delay in the inline style because I create it when opening my file. This creates the slow opening (although acceptable), which could be avoided if there wasn't any delay between rendering and styling. The issue arise as soon as I remove/add a group comment tag which must cause change potentially in the whole file. That's when you see the delay happen on scrolling (changing the visible lines)

Based on what you said in previous post, I thought you wanted me to try to create a custom EditableStyledDocument which would handle the logic of styling Python code. Did I understand correctly?

[TomasMikula]

And, in any case, I don't think the style calculation is the right conversation here. The only problem is with a delay in the display.

Correct me if I am getting this wrong, but the delay is there because the paragraphs were not styled correctly from the start, but only re-styled in reaction to a change in visible paragraphs (which is sent out only after the paragraphs are rendered).

Then all I'm trying to suggest is to get them styled correctly on the first render (not only after the visible notification).

I thought you wanted me to try to create a custom EditableStyledDocument which would handle the logic of styling Python code. Did I understand correctly?

Yes, a custom EditableStyledDocument that would return correctly styled paragraphs on the first shot.

Listening to visible paragraphs might still be useful for edits (i.e. on edit, do not restyle invisible paragraphs), but for scrolling just get the correct style from the get go.

[Symeon94]

Ok, thanks for confirming, when I'll get some time on my hand, I'll try to implement that custom document.

You are correct in your first statement (it's just that "not styled correctly" here, is just that they are not yet styled). Small correction, they are styled on 1. visibility change and 2. edition.