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

Fix table row inserted/deleted documentation and windows implementation. #515

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

szanni
Copy link

@szanni szanni commented Sep 21, 2020

As I failed to understand the documentation of both uiTableModelRowInserted() and uiTableModelRowDeleted() on when to insert to/delete from the underlying model and when to in/decrease the NumRows() counter - I decided to do some digging.

As it stands the current implementation does not seem to be so sure itself. The windows code is completely broken. The TODOs regarding API clarification and compatability with unix and darwin were never implemented. This patch set fixes all that.

Strictly API wise speaking only gtk seems to require data insertion into the model before calling uiTableModelRowInserted().
With this patch set no implementation actually relies on the value in NumRows().
But my reasoning is that both functions are past tense. The operation has already been performed and hence the state of the model should represent that. Otherwise I would consider the function names to be buggy.

Oh and I used the win32 provided macros instead of the SendMessageW((CastARoo*))((FUNCTIONS))

* Require the new row to have been added to the model
  before calling uiTableModelRowInserted.
* Require the row count to be updated before calling
  uiTableModelRowInserted.

Notes
-----

darwin: does not care about row counts internally. From the docs:
        > The numberOfRows in the table view is automatically increased
        > by the count of indexes.

unix: does not state anything regarding order in the documentation.

windows: does not state anything in the documentation but does keep
         track of its own row count internally.

So in theory we do not have to require increasing the row count before
calling uiTableModelRowInserted, but just by the very nature of the
function name (past tense) we should.
* Require the deleted row to have been removed from the
  model before calling uiTableModelRowDeleted.
* Require the row count to be updated before calling
  uiTableModelRowDeleted.

Notes
-----

darwin: Does not care about row counts internally.

unix: Requires the row to have been deleted prior to
      calling this function:
      > This should be called by models after a row has been removed.
      > The location pointed to by path should be the location that the
      > row previously was at. It may not be a valid location anymore.

windows: Does not state anything in the documentation but does keep
         track of its own row count internally.
Fix double row insertion bug on uiTableModelRowInserted().
Fix uiTableModelRowDeleted() to match API.
Remove redundant call to update row count.
Remove TODOs that have been implemented.
Use more readable win32 macros.
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.

uiTableModelInsertRow() causes invalid rows to be accessed (on windows)
1 participant