[GH-ISSUE #248 ] virtual Table view #question #192

Author

Owner

@rivo commented on GitHub (Mar 16, 2019):

At this point, no. I guess one could experiment with using a GetCell(row,column) *TableCell function that would allow you to return the cell content dynamically. But it would have to be random access. Right now, the user can jump to the beginning, to the end, page up/down, etc.. There are fixed rows/columns (at the beginning of the table). And we would need to know the total number of rows at any time. I'm not sure if you can provide that for your log. (If it's a simple file, it's probably difficult to implement without parsing it first and keeping it in memory.)

If you do have random access to your log, I could look into this. But it could take a while and I don't know yet if I might hit any major roadblocks. (I haven't looked at the code in detail from this perspective.)

@rivo commented on GitHub (Mar 16, 2019): At this point, no. I guess one could experiment with using a `GetCell(row,column) *TableCell` function that would allow you to return the cell content dynamically. But it would have to be random access. Right now, the user can jump to the beginning, to the end, page up/down, etc.. There are fixed rows/columns (at the beginning of the table). And we would need to know the total number of rows at any time. I'm not sure if you can provide that for your log. (If it's a simple file, it's probably difficult to implement without parsing it first and keeping it in memory.) If you do have random access to your log, I could look into this. But it could take a while and I don't know yet if I might hit any major roadblocks. (I haven't looked at the code in detail from this perspective.)

kerem commented

Author

Owner

@pihentagy commented on GitHub (Mar 19, 2019):

I'm thinking of parsing the logfile in the background and "index" it properly.

So I would take care of the GetCell implementation details, and telling about the number of rows/columns.

Don't get how the fixed columns/rows complicate the implementation.

But to recap the big picture: could you separate the Table model and the table view? Then I'd like to implement my own model, and pass it to your view.

@pihentagy commented on GitHub (Mar 19, 2019): I'm thinking of parsing the logfile in the background and "index" it properly. So I would take care of the GetCell implementation details, and telling about the number of rows/columns. Don't get how the fixed columns/rows complicate the implementation. But to recap the big picture: could you separate the Table model and the table view? Then I'd like to implement my own model, and pass it to your view.

kerem commented

Author

Owner

@pihentagy commented on GitHub (Mar 26, 2019):

(If it's a simple file, it's probably difficult to implement without parsing it first and keeping it in memory.)

I was thinking about parsing the logfile to an sqlite database in the background. Either way, I would just need a model interface to implement and a way to notify the viewer about the data changes.

@pihentagy commented on GitHub (Mar 26, 2019): > (If it's a simple file, it's probably difficult to implement without parsing it first and keeping it in memory.) I was thinking about parsing the logfile to an sqlite database in the background. Either way, I would just need a model interface to implement and a way to notify the viewer about the data changes.

kerem commented

Author

Owner

@rivo commented on GitHub (May 13, 2019):

Apologies for the late reply. I was away for a few weeks. Is this issue still relevant to you?

@rivo commented on GitHub (May 13, 2019): Apologies for the late reply. I was away for a few weeks. Is this issue still relevant to you?

kerem commented

Author

Owner

@pihentagy commented on GitHub (May 17, 2019):

Relevant, but I can postpone this development (as this is a half-hobby project). I haven't dealt with the problem in the past weeks, but not having a model-view separation can be a dealbreaker for me.

@pihentagy commented on GitHub (May 17, 2019): Relevant, but I can postpone this development (as this is a half-hobby project). I haven't dealt with the problem in the past weeks, but not having a model-view separation can be a dealbreaker for me.

kerem commented

Author

Owner

@pckhoi commented on GitHub (Mar 20, 2021):

What is your current appetite to see this implemented? I'm needing this and might try to implement this myself but what are the criteria you are looking for?

@pckhoi commented on GitHub (Mar 20, 2021): What is your current appetite to see this implemented? I'm needing this and might try to implement this myself but what are the criteria you are looking for?

kerem commented

Author

Owner

@pihentagy commented on GitHub (Mar 24, 2021):

To be frank I lost interest in this feature.

But the original need was to be able to lazily access a potentially huge number of items and have it rendered. So there should be a callback provided to retrieve the element specified in a parameter. It implies that the columns of the table should not depend on the actual length of the (huge) table.

@pihentagy commented on GitHub (Mar 24, 2021): To be frank I lost interest in this feature. But the original need was to be able to lazily access a potentially huge number of items and have it rendered. So there should be a callback provided to retrieve the element specified in a parameter. It implies that the columns of the table should not depend on the actual length of the (huge) table.

kerem commented

Author

Owner

@pckhoi commented on GitHub (Mar 25, 2021):

I went ahead and produced a VirtualTable that is to my liking but I'm not sure if @rivo would be interested in seeing my pull request because there are some breaking changes:

Decouple row/column selection from Table. In my opinion Table should be purely about visual and styling, and also my selection needs are different.
Add some public functions:
- func (c *TableCell) SetPosition(x, y, width int) *TableCell
Make some function public:
- printWithStyle should become PrintWithStyle
- decomposeString => DecomposeString
- iterateString => IterateString
- stringWidth => StringWidth

This would imply perhaps remake the original Table primitive on top of VirtualTable primitive and all should be well.

@pckhoi commented on GitHub (Mar 25, 2021): I went ahead and produced a VirtualTable that is to my liking but I'm not sure if @rivo would be interested in seeing my pull request because there are some breaking changes: - Decouple row/column selection from Table. In my opinion Table should be purely about visual and styling, and also my selection needs are different. - Add some public functions: - `func (c *TableCell) SetPosition(x, y, width int) *TableCell` - Make some function public: - `printWithStyle` should become `PrintWithStyle` - `decomposeString` => `DecomposeString` - `iterateString` => `IterateString` - `stringWidth` => `StringWidth` This would imply perhaps remake the original Table primitive on top of VirtualTable primitive and all should be well.

kerem commented

Author

Owner

@rivo commented on GitHub (Apr 26, 2021):

I'm not really sure I understand your suggestion. Maybe you can elaborate how this would work.

I looked at the code regarding implementing a virtual table and it doesn't look like there's a simple way. Tables are not read-only. There are all kinds of modifying functions like SetCell(), RemoveRow(), RemoveColumn(), InsertRow(), and InsertColumn(). If we wanted to back the table by some virtual structure, it would have to implement all of these functions.

Then, every time the table is redrawn, it would let the implementer know what rows can be displayed so it can adjust its mapping. While drawing, it would query the implementer for all cells in the visible row range. (If SetEvaluateAllRows() was called with true, even invisible cells of the table would be queried.)

Something like this:

type TableContent interface {
	GetCell(row, column int) *TableCell
	GetRowCount()
	GetColumnCount()

	SetCell(row, column int, cell *TableCell)
	RemoveRow(row int)
	RemoveColumn(column int)
	InsertRow(row int)
	InsertColumn(column int)
	Clear()

	UpdateView(fromRow, toRow int)
}

Looking forward to your thoughts.

@rivo commented on GitHub (Apr 26, 2021): I'm not really sure I understand your suggestion. Maybe you can elaborate how this would work. I looked at the code regarding implementing a virtual table and it doesn't look like there's a simple way. Tables are not read-only. There are all kinds of modifying functions like `SetCell()`, `RemoveRow()`, `RemoveColumn()`, `InsertRow()`, and `InsertColumn()`. If we wanted to back the table by some virtual structure, it would have to implement all of these functions. Then, every time the table is redrawn, it would let the implementer know what rows can be displayed so it can adjust its mapping. While drawing, it would query the implementer for all cells in the visible row range. (If `SetEvaluateAllRows()` was called with `true`, even invisible cells of the table would be queried.) Something like this: ```go type TableContent interface { GetCell(row, column int) *TableCell GetRowCount() GetColumnCount() SetCell(row, column int, cell *TableCell) RemoveRow(row int) RemoveColumn(column int) InsertRow(row int) InsertColumn(column int) Clear() UpdateView(fromRow, toRow int) } ``` Looking forward to your thoughts.

kerem commented

Author

Owner

@pihentagy commented on GitHub (Apr 27, 2021):

Yes that's it, separate the handling of table model (I'd use TableModel as a type) and the table view.

@pihentagy commented on GitHub (Apr 27, 2021): Yes that's it, separate the handling of table model (I'd use TableModel as a type) and the table view.

kerem commented

Author

Owner

@pckhoi commented on GitHub (Apr 27, 2021):

Your proposal assumes that the TableContent interface want to accept TableCell as input. My table has to deal with 10s of million of rows and it doesn't even keep all the raw bytes of the data let alone the table cells. Instead it take in something that is close to io.ReadSeeker and io.ReaderAt - my point is VirtualTable shouldn't deal with how data is updated. Instead you can have a Table widget that wrap VirtualTable and work with the interface that you proposed.

My implementation of VirtualTable is complicated enough so I'll only discuss how it is different from the current Table:

It allows displaying sub-columns, so for any given pair of row/col index, a list of table cells can be displayed instead of just one and all the widths are added/divided in a pleasing way.
It has a SetGetCellsFunc(getCell func(row, column int) []*TableCell) method which allow you to feed the list of cells to display at a row/col index. It doesn't keep any TableCell of course.
It doesn't handle selection nor keep any selection state. My selection need is different so I have another widget that handle selection and control offsets/styling as needed.
Doesn't have SetEvaluateAllRows.

Some of my needs such as sub-columns maybe not relevant to most people but from this VirtualTable it's possible to build the exact same Table widget.

@pckhoi commented on GitHub (Apr 27, 2021): Your proposal assumes that the `TableContent` interface want to accept `TableCell` as input. My table has to deal with 10s of million of rows and it doesn't even keep all the raw bytes of the data let alone the table cells. Instead it take in something that is close to `io.ReadSeeker` and `io.ReaderAt` - my point is `VirtualTable` shouldn't deal with how data is updated. Instead you can have a `Table` widget that wrap `VirtualTable` and work with the interface that you proposed. My implementation of `VirtualTable` is complicated enough so I'll only discuss how it is different from the current `Table`: - It allows displaying sub-columns, so for any given pair of row/col index, a list of table cells can be displayed instead of just one and all the widths are added/divided in a pleasing way. - It has a `SetGetCellsFunc(getCell func(row, column int) []*TableCell)` method which allow you to feed the list of cells to display at a row/col index. It doesn't keep any `TableCell` of course. - It doesn't handle selection nor keep any selection state. My selection need is different so I have another widget that handle selection and control offsets/styling as needed. - Doesn't have `SetEvaluateAllRows`. Some of my needs such as sub-columns maybe not relevant to most people but from this `VirtualTable` it's possible to build the exact same `Table` widget.

kerem commented

Author

Owner

@rivo commented on GitHub (May 29, 2021):

I understand your use case and if we only looked at a subset of the current feature set of Table, it would certainly be possible to implement it this way. However, I cannot simply ignore everything else that is in there and the requirements of other people who are using this package. So it's important to at least have a plan what to do with the functions I mentioned.

Sure, you can choose not to implement functions like RemoveRow(row int), that's up to you. But for those users who need these functions, I need to know what to do internally to make it work. SetGetCellsFunc(getCell func(row, column int) []*TableCell) is not enough, which is why I proposed the TableContent interface.

Along with a function Table.SetTableContent(content TableContent), you can then define your own table (view). If this is not provided (probably in the majority of cases), the default implementation of TableContent is used, which is what the table is doing now, i.e. no virtualization.

Your proposal assumes that the TableContent interface want to accept TableCell as input.

Unless I misunderstood what you wrote, I don't think this is the case. In your case, you would implement GetCell(), GetRowCount(), and GetColumnCount(). Because it's read-only, you would ignore the other functions. Only GetCell() would return a TableCell for the requested cell. (It would be wise for you to cache these because the function is potentially called very often.)

Does this make sense to you?

This is quite a bit of work so I want to make sure that we're on the same page.

@rivo commented on GitHub (May 29, 2021): I understand your use case and if we only looked at a subset of the current feature set of `Table`, it would certainly be possible to implement it this way. However, I cannot simply ignore everything else that is in there and the requirements of other people who are using this package. So it's important to at least have a plan what to do with the functions I mentioned. Sure, you can choose not to implement functions like `RemoveRow(row int)`, that's up to you. But for those users who need these functions, I need to know what to do internally to make it work. `SetGetCellsFunc(getCell func(row, column int) []*TableCell)` is not enough, which is why I proposed the `TableContent` interface. Along with a function `Table.SetTableContent(content TableContent)`, you can then define your own table (view). If this is not provided (probably in the majority of cases), the default implementation of `TableContent` is used, which is what the table is doing now, i.e. no virtualization. > Your proposal assumes that the `TableContent` interface want to accept `TableCell` as input. Unless I misunderstood what you wrote, I don't think this is the case. In your case, you would implement `GetCell()`, `GetRowCount()`, and `GetColumnCount()`. Because it's read-only, you would ignore the other functions. Only `GetCell()` would return a `TableCell` for the requested cell. (It would be wise for you to cache these because the function is potentially called very often.) Does this make sense to you? This is quite a bit of work so I want to make sure that we're on the same page.

kerem commented

Author

Owner

@pckhoi commented on GitHub (Jun 1, 2021):

I think you misunderstood some of my suggestions. Yes, my VirtualTable only access cells via SetGetCellsFunc however it doesn't mean that I can't remove row. It just means that I won't be doing that by interacting with VirtualTable directly. This is about separation of concerns. Like I said, VirtualTable should just worry about rendering cells, not how those cells contents can be changed. I mean look at Facebook's React, the store is separated from the component, if you want to update the component then you update the store, not the component directly. It absolutely doesn't mean that VirtualTable is read-only. As I said, VirtualTable does not keep any table cell but discard them all once it is done with them, but that doesn't mean that whatever is supplying those cells cannot cache them.

Again, SetGetCellsFunc's main difference compared to TableContent's methods such as GetCell and SetCell isn't so much that it is "read-only". It is about pulling updates versus pushing updates. When I want to make changes to the table, I make changes to the data directly, then I just let the VirtualTable know that it should now re-render. At which point it call the get cells func to get the cells that are currently in view. And it will result in cleaner code because all view-related logics are tucked into a few functions that deal with how to translate the object's state into the table cells whenever VirtualTable ask for them. Outside of those functions if I want to change the data I can change them directly, not having to sprinkle GetCell and SetCell all over my code.

My ideal version of TableContent interface only have get functions, and I hope you understand by now that doesn't mean it is read-only:

type TableContent interface {
	GetCells(row int, column int) []*TableCell
	GetRowCount() int
	GetColumnCount() int
}

Also I'm reiterating that I want to split the current Table widget into VirtualTable widget and a new Table widget which wrap VirtualTable and maintain current behavior. The point is to create a VirtualTable widget that only know how to display a table but otherwise stripped bare of any logic that are not strictly view-related. I was unsatisfied with some of the current Table's behaviors such as selection so this would provide an avenue to make my own table widget. Perhaps this is out of scope for this issue. If it is let me know I will open another issue.

@pckhoi commented on GitHub (Jun 1, 2021): I think you misunderstood some of my suggestions. Yes, my `VirtualTable` only access cells via `SetGetCellsFunc` however it doesn't mean that I can't remove row. It just means that I won't be doing that by interacting with `VirtualTable` directly. This is about separation of concerns. Like I said, `VirtualTable` should just worry about rendering cells, not how those cells contents can be changed. I mean look at Facebook's React, the store is separated from the component, if you want to update the component then you update the store, not the component directly. It absolutely doesn't mean that `VirtualTable` is read-only. As I said, `VirtualTable` does not keep any table cell but discard them all once it is done with them, but that doesn't mean that whatever is supplying those cells cannot cache them. Again, `SetGetCellsFunc`'s main difference compared to `TableContent`'s methods such as `GetCell` and `SetCell` isn't so much that it is "read-only". It is about **pulling updates versus pushing updates**. When I want to make changes to the table, I make changes to the data directly, then I just let the `VirtualTable` know that it should now re-render. At which point it call the get cells func to get the cells that are currently in view. And it will result in cleaner code because all view-related logics are tucked into a few functions that deal with how to translate the object's state into the table cells whenever `VirtualTable` ask for them. Outside of those functions if I want to change the data I can change them directly, not having to sprinkle `GetCell` and `SetCell` all over my code. My ideal version of `TableContent` interface only have get functions, and I hope you understand by now that doesn't mean it is read-only: ```go type TableContent interface { GetCells(row int, column int) []*TableCell GetRowCount() int GetColumnCount() int } ``` Also I'm reiterating that I want to split the current `Table` widget into `VirtualTable` widget and a new `Table` widget which wrap `VirtualTable` and maintain current behavior. The point is to create a `VirtualTable` widget that only know how to display a table but otherwise stripped bare of any logic that are not strictly view-related. I was unsatisfied with some of the current `Table`'s behaviors such as selection so this would provide an avenue to make my own table widget. Perhaps this is out of scope for this issue. If it is let me know I will open another issue.

kerem commented

Author

Owner

@rivo commented on GitHub (Jun 24, 2021):

Let me ask you this way: Assuming I have a Table backed by your VirtualTable, as you described. In your opinion, what exactly is supposed to happen when I call this function?

@rivo commented on GitHub (Jun 24, 2021): Let me ask you this way: Assuming I have a `Table` backed by your `VirtualTable`, as you described. In your opinion, what exactly is supposed to happen when I call [this function](https://pkg.go.dev/github.com/rivo/tview#Table.RemoveColumn)?

kerem commented

Author

Owner

@pckhoi commented on GitHub (Jun 25, 2021):

I assume you want the interface for Table remain unchanged. Which means the table need to keep all of its cells in a two-dimension matrix along with row and column count. Which is something that the current Table is already doing. The new function will be exactly the same as your current Table.RemoveColumn function. The difference is, this new Table type will not have any drawing logic whatsoever, it merely know how many rows there are, how many columns there are, what cell is at a certain position. The VirtualTable will take care of drawing, Table can tell it how many row and column there are via SetShape or even SetShapeGetter, doesn't matter. The Table can also provide it with a function that it can call whenever it needs a cell at a certain position via SetGetCellsFunc.

And then when the Table.Draw is called it can simply call VirtualTable.Draw because VirtualTable is embedded. VirtualTable calls its internal .getCells function (which was provided via SetGetCellsFunc) to get the cells that it need to draw. It will be very straight forward.

@pckhoi commented on GitHub (Jun 25, 2021): I assume you want the interface for `Table` remain unchanged. Which means the table need to keep all of its cells in a two-dimension matrix along with row and column count. Which is something that the current `Table` is already doing. The new function will be exactly the same as your current `Table.RemoveColumn` function. The difference is, this new `Table` type will not have any drawing logic whatsoever, it merely know how many rows there are, how many columns there are, what cell is at a certain position. The `VirtualTable` will take care of drawing, `Table` can tell it how many row and column there are via `SetShape` or even `SetShapeGetter`, doesn't matter. The `Table` can also provide it with a function that it can call whenever it needs a cell at a certain position via `SetGetCellsFunc`. And then when the `Table.Draw` is called it can simply call `VirtualTable.Draw` because `VirtualTable` is embedded. `VirtualTable` calls its internal `.getCells` function (which was provided via `SetGetCellsFunc`) to get the cells that it need to draw. It will be very straight forward.

kerem commented

Author

Owner

@marcelocantos commented on GitHub (Jun 25, 2021):

Why not just implement VirtualTable as a new widget type distinct from Table? It could perhaps work by subclassing Table to leverage some of its code, but that would be an implementation detail. Table itself would remain as is.

@marcelocantos commented on GitHub (Jun 25, 2021): Why not just implement `VirtualTable` as a new widget type distinct from `Table`? It could perhaps work by subclassing `Table` to leverage some of its code, but that would be an implementation detail. `Table` itself would remain as is.

kerem commented

Author

Owner

@pckhoi commented on GitHub (Jun 25, 2021):

Because that means code duplication. Any drawing logic that Table possess VirtualTable will surely have it as well. By making Table relying on VirtualTable for drawing, any change to drawing logic only need to be made at one place.

@pckhoi commented on GitHub (Jun 25, 2021): Because that means code duplication. Any drawing logic that `Table` possess `VirtualTable` will surely have it as well. By making `Table` relying on `VirtualTable` for drawing, any change to drawing logic only need to be made at one place.

kerem commented

Author

Owner

@pckhoi commented on GitHub (Jun 25, 2021):

But if you are simply asking whether VirtualTable is a separate widget from Table then yes, it is a separate, lower-level widget.

@pckhoi commented on GitHub (Jun 25, 2021): But if you are simply asking whether `VirtualTable` is a separate widget from `Table` then yes, it is a separate, lower-level widget.

kerem commented

Author

Owner

@pckhoi commented on GitHub (Jun 25, 2021):

As for what type should be embedded in what type, I thought the answer should be VirtualTable as the embedded widget because it is lower-level.

@pckhoi commented on GitHub (Jun 25, 2021): As for what type should be embedded in what type, I thought the answer should be `VirtualTable` as the embedded widget because it is lower-level.

kerem commented

Author

Owner

@marcelocantos commented on GitHub (Jun 25, 2021):

It would make sense to embed VirtualTable if Table wasn't already coded without it. As it stands, going the other way is cleaner, since it doesn't require changing an existing implementation that wasn't designed with virtualisation of its contents in mind.

I don't know the code base very well, but I suspect you could reuse most of the Table code by embedding it in VirtualTable.

@marcelocantos commented on GitHub (Jun 25, 2021): It would make sense to embed `VirtualTable` if `Table` wasn't already coded without it. As it stands, going the other way is cleaner, since it doesn't require changing an existing implementation that wasn't designed with virtualisation of its contents in mind. I don't know the code base very well, but I suspect you could reuse most of the `Table` code by embedding it in `VirtualTable`.

kerem commented

Author

Owner

@pckhoi commented on GitHub (Jun 25, 2021):

I was never aware that putting in the effort is one of the issues in discussion here. But if @rivo decides that he likes this direction then rest assure I'll open a PR with my own VirtualTable and Table implementation. I have already been using my own VirtualTable rewrite for a while now.

@pckhoi commented on GitHub (Jun 25, 2021): I was never aware that putting in the effort is one of the issues in discussion here. But if @rivo decides that he likes this direction then rest assure I'll open a PR with my own `VirtualTable` and `Table` implementation. I have already been using my own `VirtualTable` rewrite for a while now.

kerem commented

Author

Owner

@rivo commented on GitHub (Jun 25, 2021):

I assume you want the interface for Table remain unchanged.

Definitely yes. This is one of the core principles of this package that whenever possible, backwards compatibility is guaranteed.

@pckhoi I've gone over your comments many times. Maybe I'm looking at the problem from a completely different angle but I have trouble understanding the details of your suggestion, especially how to handle write operations when the Table class itself doesn't have direct access to the data anymore (just via the read-only functions you suggested).

I have a slight hunch that you want the Table class to map visible cell positions (the ones on screen) to original cell positions (the ones in your own data structure). If that's the case, then for very large tables — and this is what the original request was about — Table would again create a very large matrix of position mappings. That wouldn't be desirable, in my opinion. Maybe you mean something else?

But since you already have an implementation, how about you post a link to it? No need to submit a PR, a link to your code will do. I'll have a look. Maybe that will help me understand where you're coming from.

@rivo commented on GitHub (Jun 25, 2021): > I assume you want the interface for `Table` remain unchanged. Definitely yes. This is one of the core principles of this package that whenever possible, backwards compatibility is guaranteed. @pckhoi I've gone over your comments many times. Maybe I'm looking at the problem from a completely different angle but I have trouble understanding the details of your suggestion, especially how to handle write operations when the `Table` class itself doesn't have direct access to the data anymore (just via the read-only functions you suggested). I have a slight hunch that you want the `Table` class to map visible cell positions (the ones on screen) to original cell positions (the ones in your own data structure). If that's the case, then for very large tables — and this is what the original request was about — `Table` would again create a very large matrix of position mappings. That wouldn't be desirable, in my opinion. Maybe you mean something else? But since you already have an implementation, how about you post a link to it? No need to submit a PR, a link to your code will do. I'll have a look. Maybe that will help me understand where you're coming from.

kerem commented

Author

Owner

@pckhoi commented on GitHub (Jun 25, 2021):

My table would not need to maintain a large matrix of TableCell, not even all the bytes are needed in memory. But it does maintain a cache of visible cells which get updated or removed depending on what positions are requested. In any case, look in this package there's a VirtualTable and other kinds of tables which all depend on VirtualTable for actual drawing.

@pckhoi commented on GitHub (Jun 25, 2021): My table would not need to maintain a large matrix of `TableCell`, not even all the bytes are needed in memory. But it does maintain a cache of visible cells which get updated or removed depending on what positions are requested. In any case, look in this [package](https://github.com/wrgl/core/tree/wrgld/pkg/widgets) there's a VirtualTable and other kinds of tables which all depend on `VirtualTable` for actual drawing.

kerem commented

Author

Owner

@pckhoi commented on GitHub (Jun 25, 2021):

If you're asking about whether a new Table implementation with identical interface that embed VirtualTable, would be better at handling large matrix of cells then no. Because the old interface dictates that Table must keep all of its cells in memory, visible or not. The point of VirtualTable then is to enable other kinds of table that can actually handle infinite amount of data to be built on top of it.

@pckhoi commented on GitHub (Jun 25, 2021): If you're asking about whether a new `Table` implementation with identical interface that embed `VirtualTable`, would be better at handling large matrix of cells then no. Because the old interface dictates that `Table` must keep all of its cells in memory, visible or not. The point of `VirtualTable` then is to enable other kinds of table that can actually handle infinite amount of data to be built on top of it.

kerem commented

Author

Owner

@rivo commented on GitHub (Jun 25, 2021):

Thanks for providing a link to your implementation. It helped me understand what you've been talking about all along. (Maybe it's just me but from your descriptions alone, it was not clear that this is what you meant — too many questions unanswered.)

To be honest, though, I find this distinction between your VirtualTable and the Table not clear. Sure enough, VirtualTable implements a virtual table but only the read-only parts. You say that if someone also wants write access, they should just modify the underlying data structure. Well, this means they then have to expose their data structure in a separate way somehow because there are no write methods in VirtualTable. Or, alternatively, they could just use Table which implements them using a specific implementation of VirtualTable. But then they're stuck with Table which would be pretty much the way it is now and offers none of virtualization we want.

I also don't think this distinction between VirtualTable and Table would be clear to many people. And ultimately, it's not consistent with how extensibility is implemented in the tview package. Almost every tview widget provides specific functionality out of the box, and if you want to modify/extend that functionality, you "plug it in" with functions like SetInputCapture() or SetAutocompleteFunc(). There is no separate class VirtualInputField for extra InputField functionality.

So all in all, I think what I outlined before is what I'll be going for after all. You will still be able to handle changes to the data on the implementation side, i.e. your "separation of concerns". For example, if you want to display a log that is continuously being written to, you're free to let the table access only the trailing rows of your log. You don't need to call or even implement RemoveRow() to make that happen. But for those users who want to provide a Table with all of its functionality but with a different data structure, they can do it using the interface described above.

I understand that this is a very different direction from the one you've already taken and you may not be interested in this version. There are other people in this thread, though, that will hopefully benefit from this. (Also, I would say that things like sub-cells, the lack of the "evaluate all rows" flag, and a different selection functionality are out of scope for now anyway.)

Everyone, feel free to weigh in. I'm never quite sure if I missed a certain perspective that would make this a bad solution. But as of now, it looks like this will be the implementation.

@rivo commented on GitHub (Jun 25, 2021): Thanks for providing a link to your implementation. It helped me understand what you've been talking about all along. (Maybe it's just me but from your descriptions alone, it was not clear that this is what you meant — too many questions unanswered.) To be honest, though, I find this distinction between your `VirtualTable` and the `Table` not clear. Sure enough, `VirtualTable` implements a virtual table but only the read-only parts. You say that if someone also wants write access, they should just modify the underlying data structure. Well, this means they then have to expose their data structure in a separate way somehow because there are no write methods in `VirtualTable`. Or, alternatively, they could just use `Table` which implements them using a specific implementation of `VirtualTable`. But then they're stuck with `Table` which would be pretty much the way it is now and offers none of virtualization we want. I also don't think this distinction between `VirtualTable` and `Table` would be clear to many people. And ultimately, it's not consistent with how extensibility is implemented in the `tview` package. Almost every `tview` widget provides specific functionality out of the box, and if you want to modify/extend that functionality, you "plug it in" with functions like `SetInputCapture()` or `SetAutocompleteFunc()`. There is no separate class `VirtualInputField` for extra `InputField` functionality. So all in all, I think what I [outlined before](https://github.com/rivo/tview/issues/248#issuecomment-827005268) is what I'll be going for after all. You will still be able to handle changes to the data on the implementation side, i.e. your "separation of concerns". For example, if you want to display a log that is continuously being written to, you're free to let the table access only the trailing rows of your log. You don't need to call or even implement `RemoveRow()` to make that happen. But for those users who want to provide a `Table` with all of its functionality but with a different data structure, they can do it using the interface described above. I understand that this is a very different direction from the one you've already taken and you may not be interested in this version. There are other people in this thread, though, that will hopefully benefit from this. (Also, I would say that things like sub-cells, the lack of the "evaluate all rows" flag, and a different selection functionality are out of scope for now anyway.) Everyone, feel free to weigh in. I'm never quite sure if I missed a certain perspective that would make this a bad solution. But as of now, it looks like this will be the implementation.

kerem commented

Author

Owner

@pckhoi commented on GitHub (Jun 26, 2021):

I see that you have decided on an outcome. I don't think I can change your mind but the VirtualTable does follow a consistent pattern of the tview package that is composability. Higher-level components can be composed of lower-level components. For example 99% of all users are never expected to use the Box component, yet it is still exposed. And this is a powerful pattern, adding functionalities using layers of components (which is very successful in Facebook's React ecosystem). Extensibility via plugins is certainly possible but it is a dying trend, just look at React and a slew of technologies that emulate it such as Web Component, Vue.js. None of them rely on plugins for extensibility, all rely on composability. Since tview is a view library I thought it would be best if it also learns from other successful view libraries. But I understand if you don't agree.

@pckhoi commented on GitHub (Jun 26, 2021): I see that you have decided on an outcome. I don't think I can change your mind but the VirtualTable does follow a consistent pattern of the tview package that is composability. Higher-level components can be composed of lower-level components. For example 99% of all users are never expected to use the Box component, yet it is still exposed. And this is a powerful pattern, adding functionalities using layers of components (which is very successful in Facebook's React ecosystem). Extensibility via plugins is certainly possible but it is a dying trend, just look at React and a slew of technologies that emulate it such as Web Component, Vue.js. None of them rely on plugins for extensibility, all rely on composability. Since tview is a view library I thought it would be best if it also learns from other successful view libraries. But I understand if you don't agree.

kerem commented

Author

Owner

@pckhoi commented on GitHub (Jun 26, 2021):

And regarding modifying data away from the table. Again, this is a pattern borrowed from React: separation of state management and view logic. But anyway if you feel like it's just coming from a React fanboy then feel free to ignore me.

@pckhoi commented on GitHub (Jun 26, 2021): And regarding modifying data away from the table. Again, this is a pattern borrowed from React: separation of state management and view logic. But anyway if you feel like it's just coming from a React fanboy then feel free to ignore me.

kerem commented

Author

Owner

@rivo commented on GitHub (Jun 26, 2021):

Sure, React has its merits, and it's currently undoubtedly popular. Whether or not its model is the best of its kind is debatable. I've been programming with UI toolkits for 25 years, including React, and honestly, I wouldn't say React is the best I've ever used. In fact, when I started using it, I found it quite difficult to get it right due to its variety of concepts (e.g. mounting, updating, unmounting pre/post phases, states vs. props, and many others). I wanted tview to be very easy to get started with.

What I didn't anticipate was that people would request a lot more complex functionality from tview than what it already does. So when people needed to customize their widgets, I added ways to do it here and there, and it ended up being the plug-in kind of architecture. We're here now. And in packages like this one, I value consistency more than going with the latest trend. (Which, incidentally, is why I love Go.)

As a side note, I don't see how my proposal mixes state management and view logic. Whatever your data structure is, a Table won't know about it and if you make changes to your data structure, Table will reflect that. The question whether there have to be two different classes (VirtualTable and Table) rather than one (Table) is a different one, in my opinion.

99% of all users are never expected to use the Box component, yet it is still exposed.

Oh, I would love to hide Box from users (and quite a few other classes/interfaces, by the way). But there are no abstract classes in Go. And I already mentioned elsewhere that I don't think replicating all methods in all classes is a good idea. (Also, I initially made the mistake of making too much public, and now I can't go back.)

@rivo commented on GitHub (Jun 26, 2021): Sure, React has its merits, and it's currently undoubtedly popular. Whether or not its model is the best of its kind is debatable. I've been programming with UI toolkits for 25 years, including React, and honestly, I wouldn't say React is the best I've ever used. In fact, when I started using it, I found it quite difficult to get it right due to its variety of concepts (e.g. mounting, updating, unmounting pre/post phases, states vs. props, and many others). I wanted `tview` to be very easy to get started with. What I didn't anticipate was that people would request a lot more complex functionality from `tview` than what it already does. So when people needed to customize their widgets, I added ways to do it here and there, and it ended up being the plug-in kind of architecture. We're here now. And in packages like this one, I value consistency more than going with the latest trend. (Which, incidentally, is why I love Go.) As a side note, I don't see how my proposal mixes state management and view logic. Whatever your data structure is, a `Table` won't know about it and if you make changes to your data structure, `Table` will reflect that. The question whether there have to be two different classes (`VirtualTable` and `Table`) rather than one (`Table`) is a different one, in my opinion. > 99% of all users are never expected to use the Box component, yet it is still exposed. Oh, I would love to hide `Box` from users (and quite a few other classes/interfaces, by the way). But there are no abstract classes in Go. And I already mentioned elsewhere that I don't think replicating all methods in all classes is a good idea. (Also, I initially made the mistake of making too much public, and now I can't go back.)

kerem commented

Author

Owner

@marcelocantos commented on GitHub (Jun 27, 2021):

TableView is fundamentally a read-only object. Changes are expressed by modifying the source data and telling TableView that something changed so it can refresh the screen. Caching and localising the blast radius of a change (avoid reporting changes to a cell when it's not view) are key design questions.

This is why I disagree with the idea of introducing virtualisation to Table. It's a poor fit.

My original thought was that, if Table isn't easy to use as the backend for VirtualTable, then perhaps some additions to the API could make it so. On reflection, however, I suspect that that's not practical. The design considerations are quite different. For instance, a VirtualTable could in theory have a billion rows, each with its own size (Is size a per-row variable in Table?). You really don't want to create a billion placeholder rows, though. Moreover, you're much more likely to want to filter on a billion-row table than scroll through it (or at the very least filter before scrolling).

I think VirtualTable would be a fine addition to the library, but I strongly suspect that it'll require its own implementation distinct from Table. That said, once implemented, Table could be recast quite easily as a thin shim over VirtualTable.

@marcelocantos commented on GitHub (Jun 27, 2021): `TableView` is fundamentally a read-only object. Changes are expressed by modifying the source data and telling `TableView` that something changed so it can refresh the screen. Caching and localising the blast radius of a change (avoid reporting changes to a cell when it's not view) are key design questions. This is why I disagree with the idea of introducing virtualisation to `Table`. It's a poor fit. My original thought was that, if `Table` isn't easy to use as the backend for `VirtualTable`, then perhaps some additions to the API could make it so. On reflection, however, I suspect that that's not practical. The design considerations are quite different. For instance, a `VirtualTable` could in theory have a billion rows, each with its own size (Is size a per-row variable in `Table`?). You really don't want to create a billion placeholder rows, though. Moreover, you're much more likely to want to filter on a billion-row table than scroll through it (or at the very least filter before scrolling). I think `VirtualTable` would be a fine addition to the library, but I strongly suspect that it'll require its own implementation distinct from `Table`. That said, once implemented, `Table` could be recast quite easily as a thin shim over `VirtualTable`.

kerem commented

Author

Owner

@rivo commented on GitHub (Aug 7, 2021):

So this is implemented now. But for now, it's in a separate branch "virtualtable" (see PR #634). I also added an example with demos/table/virtualtable/main.go.

As discussed previously, Table's write functions are still there for compatibility reasons. But if you want to ignore them and instead modify the table on your end, i.e. making Table a read-only object, you're free to do so. The TableContentReadOnly struct will help you reduce your boilerplate in that case.

Give it a go and let me know how it works for you. Once it's stable, I'll merge the PR into the master branch.

@rivo commented on GitHub (Aug 7, 2021): So this is implemented now. But for now, it's in a separate branch "virtualtable" (see PR #634). I also added an example with `demos/table/virtualtable/main.go`. As discussed previously, `Table`'s write functions are still there for compatibility reasons. But if you want to ignore them and instead modify the table on your end, i.e. making `Table` a read-only object, you're free to do so. The `TableContentReadOnly` struct will help you reduce your boilerplate in that case. Give it a go and let me know how it works for you. Once it's stable, I'll merge the PR into the master branch.

kerem commented