Re: [lmi] Correctness and performance of varying column width mode in the census view

[Vadim Zeitlin]

On Fri, 29 May 2020 01:24:18 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 2020-05-28 23:52, Vadim Zeitlin wrote:
GC> > On Thu, 28 May 2020 22:02:04 +0000 Greg Chicares 
<gchicares@sbcglobal.net> wrote:
GC> [...]
GC> > GC> Entering strings shouldn't cause the column width to change at all.
GC> > 
GC> >  I understand that truncating (actually, ellipsizing) strings is not as 
bad
GC> > as doing it for the numbers, but couldn't we still expand the columns 
width
GC> > for them just for consistency, both of the UI and the implementation? The
GC> > latter shouldn't dictate the former, of course, but it's simpler to do the
GC> > same thing for all cells, so if it's not actively harmful, why not do it
GC> > for the strings too?
GC> 
GC> Okay. I suppose that if I edit a cell to change a person's name, and the
GC> new name is longer than the old width, then it's natural for lmi to widen
GC> the column.

 Yes, this is exactly what I think.

GC> You mention ellipsizing. I think I like truncating better. And 'excel'
GC> only truncates--it never ellipsizes AFAIK--so ellipsizing is "nonstandard".

 Oh. We've implemented ellipsizing support in wxGrid because this is how
the existing wxDVC-based version works and while it's, of course, not a
problem to turn it off now (it's literally a one line change in lmi code),
this is still quite surprising to hear because I also remembered/thought
that using ellipsization in the existing version was a conscious choice.
As always, it's possible that I remembered wrong, but I definitely like
using it more than just truncating personally. So I'd still like to ask you
to test the current version, using ellipsization, before deciding whether
you really want to turn it off. IMO readability is much better with it, but
the choice is yours, of course.

 Out of curiosity, has anybody ever complained about or even just commented
on the user of ellipses in wxDVC during all these years it was in use?

GC> > GC> Instead, I think we should adjust columns widths in this way:
GC> > GC>  - if one string cell changes, do nothing: O(0)
GC> > GC>  - if one numeric cell changes, widen its column if needed: O(1)
GC> > GC>    do that whether the new value is typed in or pasted in
GC> > GC>  - census pasting and applying cell or class changes seem to be
GC> > GC>    O(#rows * #columns), so that's the tough case that requires
GC> > GC>    more investigation and thought--a full resize would be nice,
GC> > GC>    but might be too costly
GC> 
GC> There's another important scenario: pop up the tabbed dialog to edit
GC> a single cell, and press OK after making some changes. That should take
GC> O(# of data changed in tabbed dialog) time. 

 Yes, indeed, thanks for reminding me about this case. We definitely need
to optimize computing the size of any newly shown column, we shouldn't be
iterating over N rows when the column has (AFAIK, please correct me if I'm
wrong) just 2 different values, whatever the value of N: the one entered
for the current cell and the default one used for all the other ones.

[strings]
GC> I guess they should all be treated the same way. But that's the
GC> simplest thing to do, if we automatically widen columns as needed
GC> for string as well as numeric input, because then all datatypes
GC> are treated the same way.

 Yes, so I'm perfectly happy with this choice.

GC> > GC> AFAICT, if you delete an 'excel' column, the widths of the remaining
GC> > GC> columns are unaffected. That sounds just right.
GC> > 
GC> >  Just to be clear, I meant deleting the rows. In this case finding the
GC> > perfectly correct new best width is O(N), as you could have deleted the 
row
GC> > containing the widest value for this column.
GC> 
GC> Then either you invest heavily in making it fast, by caching the
GC> width of every cell in the grid, and pruning the cache entries for
GC> deleted rows (but that sounds like overkill); or you let it be
GC> really slow; or you just ignore row deletions, which would be my
GC> suggestion.

 Again, I can only agree.

 To (re)summarize, I'm going to disable implicit auto-sizing for now and
let you integrate the initial version of the patch before implementing the
rest of the improvements/optimizations discussed here, which probably won't
happen immediately, but at least I think I understand what needs to be done
pretty well by now.

 Thanks!
VZ

pgp7XeA50O6Ht.pgp
Description: PGP signature