Re: [Nano-devel] Softwrap navigation overhaul

[David Ramsey]

On Tue, Jan 31, 2017 at 2:54 PM, Benno Schulenberg
<address@hidden> wrote:
> With single patch I mean: all the miscellaneous comment tweaks and
> corrections.  A coordinated logical change, like patch 37, should stay
> a separate patch, of course.

Since some of the changes seem to be a bit controversial, I'll keep them
separate until they're cleared (there aren't many of them).

> You can keep it two separate patches, if you like: one for removal,
> and one for addition of the right stuff.  But it would be nice if the
> second would come right after the first.

Hmmm.  The ad hoc bits do get in the way of the current code if I leave
them in, but I'll work something out.

> Yes, it's all the strlenpt() calls.  And mainly because most of them
> start from the head of the line instead of from a known point that is
> on the left edge of the screen.

Indeed.

> What does it do then?  After 1024 characters, a line just runs off the
> screen?  And when you go to character 1200 it scrolls horizontally?

It wraps the lines and sets the file to read-only mode, which I don't
think we want to do.

> If home and end used flowing mode, they would do the right thing.
> Apparently focusing is TRUE here, when it should be FALSE.

Which is what I meant.  edit_redraw() doesn't use stationary mode, while
edit_refresh() does, so we want the latter, and focusing has to be FALSE
to get stationary mode.

> They are not okay.  I just didn't get around to changing them. If you
> want to know whether I "approve" of something, look at git blame.  If
> I changed the line, it is most likely in my style. If I never touched
> it, it doesn't mean that I approve.

Understood.  I'll let you handle adjusting those parts, then.

> They didn't paper over the problems, they fixed them.  They weren't a
> general solution for all of the softwrap problems, but if each of the
> problems had been tackled in this way, it would have been a complete
> solution.

True.  It's just the ad hoc solutions piling on top of each other get to
be a problem when trying to come up with a general solution.  If I came
off as offensive here, it's not what I intended.

> Sure, it couldn't handle a softwrapped line that fills more than the
> screen -- but, really, who wants to handle such giant lines?

True, that's a rare case, but the editor shouldn't break then either.

> You mean: if nrows is zero or less?  Okay, but I would still do it
> outside the for loop: if (nrows > 0) initialize.

Okay.  The problem as it stands is moot, though; I'm looking into fixing
it another way.

> In your comments you sometimes say what is going to be assigned to
> what variable: you retell the code in words.  There is no need for
> that: the code already shows what is being done with the variables.
> The comments (if any are needed) should be at least one level
> "higher".

I'll keep that in mind; thanks.

> If necessary, yes.

Okay.

> Loading a file of two hundred thousand lines takes some three seconds
> on my machine.  An integer can count up to two billion lines --
> loading a file of that size would take eight hours, and require eighty
> gigabytes of memory (when an avarage line is forty characters long).
> So... the chances we'll run into that problem... will become
> interesting in maybe ten years time.

Okay.  I suppose I'm just overly paranoid when it comes to variable
types.

> No.  Something stlll being there doesn't mean that I like it. It just
> means that I don't know how to get rid of it.  There is a comment
> somewhere in one of the now-closed softwrap bugs on Savannah where I
> protest against the maxlines mechanism (then still maxrows).  And in
> debugging output I've often seen compute_maxlines() being called twice
> in a row, completely unnecessarily.  So I hated that function, but
> never got around to stamping it out.

Understood.  With luck and coding skills, it should be stamped out soon
enough :)

> Hm.  Good one.  :)  Well, they are chunks too, just slightly
> smaller ones, because the "pages" overlap by six characters
> (eight, if the dollar signs weren't getting displayed).  So
> I would add to the "horizontally within a line in chunks":
> "(a bit smaller than the chunks used in softwrapping)".

Done.

> Sounds good.

Okay.

> But somewhere you add line_x to mouse_x, and mouse_x is a
> column position:
>
>             openfile->current_x = actual_x(openfile->current->data,
>                                         line_x + mouse_x);

It would be nice if, in the long term, there was a better way to
differentiate between character and column positions, to avoid all this
confusion.

> True.  It is more complicated than I at first thought.

I'm looking into it.