* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible @ 2015-07-08 18:10 Michael Heerdegen 2015-07-08 20:03 ` Eli Zaretskii 2015-10-07 6:34 ` bug#21012: Close Michael Heerdegen 0 siblings, 2 replies; 38+ messages in thread From: Michael Heerdegen @ 2015-07-08 18:10 UTC (permalink / raw) To: 21012 [-- Attachment #1: Type: text/plain, Size: 182 bytes --] Hello! in eww, using shr-use-fonts -> t, for some lines, the last character is only partially visible. Not a big deal, but distracting. Here is a screenshot made with emacs -Q: [-- Attachment #2: eww.png --] [-- Type: image/png, Size: 61775 bytes --] [-- Attachment #3: Type: text/plain, Size: 2693 bytes --] The issue seems to happen more likely with larger fonts (e.g. with text-scale-mode -- of course I had hitten g after rescaling), but it also happens with the default font here. While browsing through and playing with the code, I found two places where I could improve things: 1. --8<---------------cut here---------------start------------->8--- (defun shr-insert-document (dom) ... (setq shr-content-cache nil) (let ((start (point)) (shr-start nil) ... (shr-internal-width (or (and shr-width..3..) (if (not shr-use-fonts) (- (window-width) 2) (- (window-pixel-width) ; <---- here (* (frame-fringe-width) 2)))))) (shr-descend dom) (shr-fill-lines start (point)) (shr-remove-trailing-whitespace start (point)) (when shr-warning..1..))) --8<---------------cut here---------------end--------------->8--- AFAICT (- (window-pixel-width) (* (frame-fringe-width) 2)) is not the available width for text, it is a larger value including scroll bars etc. When I change it to (window-body-width nil t) this improves things. The issue still occurs with this change, though much less often. 2. --8<---------------cut here---------------start------------->8--- (defun shr-vertical-motion (column) (if (not shr-use-fonts) (move-to-column column) (unless (eolp) (forward-char 1)) (vertical-motion (cons (/ column (frame-char-width)) 0)) ; <-- here (unless (eolp) (forward-char 1)))) --8<---------------cut here---------------end--------------->8--- This function is used, among other places, to decide where to break lines in `shr-fill-line'. Probably (/ column (frame-char-width)) can be too large if you are unlucky. For testing I tried with this version: --8<---------------cut here---------------start------------->8--- (defun shr-vertical-motion (column) (if (not shr-use-fonts) (move-to-column column) (unless (eolp) (forward-char 1)) (end-of-visual-line))) --8<---------------cut here---------------end--------------->8--- This seems to fix this issue (together with the first change), though I guess it's wrong when shr-vertical-motion is called with column < window-width (dunno if this is done somewhere). Thanks, Michael. In GNU Emacs 25.0.50.4 (x86_64-unknown-linux-gnu, GTK+ Version 3.16.4) of 2015-07-07 on drachen Repository revision: 0bfc94047da4960af55196242728a7a55120867f Windowing system distributor `The X.Org Foundation', version 11.0.11702000 System Description: Debian GNU/Linux testing (stretch) Configured features: XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND DBUS GSETTINGS NOTIFY LIBXML2 FREETYPE XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-08 18:10 bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible Michael Heerdegen @ 2015-07-08 20:03 ` Eli Zaretskii 2015-07-08 20:24 ` Michael Heerdegen 2015-07-08 20:31 ` Michael Heerdegen 2015-10-07 6:34 ` bug#21012: Close Michael Heerdegen 1 sibling, 2 replies; 38+ messages in thread From: Eli Zaretskii @ 2015-07-08 20:03 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Date: Wed, 08 Jul 2015 20:10:35 +0200 > > in eww, using shr-use-fonts -> t, for some lines, the last character is > only partially visible. Not a big deal, but distracting. Is the partially-visible character always the last character on that line, before the hard newline? If so, I think I know where's the flaw in the logic. See below. > --8<---------------cut here---------------start------------->8--- > (defun shr-insert-document (dom) > ... > (setq shr-content-cache nil) > (let ((start (point)) > (shr-start nil) > ... > (shr-internal-width (or (and shr-width..3..) > (if (not shr-use-fonts) > (- (window-width) 2) > (- (window-pixel-width) ; <---- here > (* (frame-fringe-width) 2)))))) > (shr-descend dom) > (shr-fill-lines start (point)) > (shr-remove-trailing-whitespace start (point)) > (when shr-warning..1..))) > --8<---------------cut here---------------end--------------->8--- > > AFAICT > > (- (window-pixel-width) (* (frame-fringe-width) 2)) > > is not the available width for text, it is a larger value including > scroll bars etc. Do you understand why the value of frame-fringe-width is multiplied by 2? > When I change it to > > (window-body-width nil t) > > this improves things. I think that using window-body-width is indeed better here. It will, for example, account for display margins. > --8<---------------cut here---------------start------------->8--- > (defun shr-vertical-motion (column) > (if (not shr-use-fonts) > (move-to-column column) > (unless (eolp) > (forward-char 1)) > (vertical-motion (cons (/ column (frame-char-width)) 0)) ; <-- here > (unless (eolp) > (forward-char 1)))) > --8<---------------cut here---------------end--------------->8--- > > This function is used, among other places, to decide where to break > lines in `shr-fill-line'. > > Probably (/ column (frame-char-width)) can be too large if you are > unlucky. Sorry, I don't follow. Can you elaborate on when this could happen? > For testing I tried with this version: > > --8<---------------cut here---------------start------------->8--- > (defun shr-vertical-motion (column) > (if (not shr-use-fonts) > (move-to-column column) > (unless (eolp) > (forward-char 1)) > (end-of-visual-line))) > --8<---------------cut here---------------end--------------->8--- > > This seems to fix this issue (together with the first change), I don't see how this could be right, unless you only tested it with text that is rendered using a single font. move-to-column goes to the Nth character starting from the left edge (forget tabs and double-width CJK characters for a moment), so it will not DTRT when a screen line displays characters of different size (as in with different-size fonts). The original code works in pixels (vertical-motion interprets the column number as the number of pixels equivalent to that number of frame's canonical characters), so it is not prone to this problem. I believe the problem is that the code determines whether the line should be wrapped based on this test: (shr-vertical-motion shr-internal-width) (when (looking-at " $") <<<<<<<<<<<<<<<<<<<<<<<<< (delete-region (point) (line-end-position))) IOW, it assumes that if the character at the goal pixel coordinate immediately precedes the newline, the line doesn't need to be wrapped. But that will fail if that last character is unusually wide. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-08 20:03 ` Eli Zaretskii @ 2015-07-08 20:24 ` Michael Heerdegen 2015-07-09 2:38 ` Eli Zaretskii 2015-07-08 20:31 ` Michael Heerdegen 1 sibling, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-07-08 20:24 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21012 Eli Zaretskii <eliz@gnu.org> writes: > Is the partially-visible character always the last character on that > line, before the hard newline? From what I saw - yes, but I'm not if "always" is correct. > > (- (window-pixel-width) (* (frame-fringe-width) 2)) > > > > is not the available width for text, it is a larger value including > > scroll bars etc. > > Do you understand why the value of frame-fringe-width is multiplied by > 2? I guess because the window is assumed to have two fringes. > I think that using window-body-width is indeed better here. It will, > for example, account for display margins. Yes, I think the author of shr just tried to reinvent it. > > --8<---------------cut here---------------start------------->8--- > > (defun shr-vertical-motion (column) > > (if (not shr-use-fonts) > > (move-to-column column) > > (unless (eolp) > > (forward-char 1)) > > (vertical-motion (cons (/ column (frame-char-width)) 0)) ; <-- here > > (unless (eolp) > > (forward-char 1)))) > > --8<---------------cut here---------------end--------------->8--- > > > > This function is used, among other places, to decide where to break > > lines in `shr-fill-line'. > > > > Probably (/ column (frame-char-width)) can be too large if you are > > unlucky. > > Sorry, I don't follow. Can you elaborate on when this could happen? Note that in the shr-use-fonts -> t case, COLUMN is in pixels. (/ column (frame-char-width)) is IMO an estimated value of "real" columns to advance. But if there are many wide characters, this can be too large. > > For testing I tried with this version: > > > > --8<---------------cut here---------------start------------->8--- > > (defun shr-vertical-motion (column) > > (if (not shr-use-fonts) > > (move-to-column column) > > (unless (eolp) > > (forward-char 1)) > > (end-of-visual-line))) > > --8<---------------cut here---------------end--------------->8--- > > > > This seems to fix this issue (together with the first change), > > I don't see how this could be right, unless you only tested it with > text that is rendered using a single font. move-to-column goes to the > [...] I think you looked at the wrong `if' branch...? What I changed was to use `end-of-visual-line', which I hope is more accurate than (vertical-motion (cons (/ column (frame-char-width)) 0)) for finding the right point for breaking the line. Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-08 20:24 ` Michael Heerdegen @ 2015-07-09 2:38 ` Eli Zaretskii 2015-07-09 11:01 ` Michael Heerdegen 2015-07-09 15:34 ` Eli Zaretskii 0 siblings, 2 replies; 38+ messages in thread From: Eli Zaretskii @ 2015-07-09 2:38 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: 21012@debbugs.gnu.org > Date: Wed, 08 Jul 2015 22:24:21 +0200 > > > > (- (window-pixel-width) (* (frame-fringe-width) 2)) > > > > > > is not the available width for text, it is a larger value including > > > scroll bars etc. > > > > Do you understand why the value of frame-fringe-width is multiplied by > > 2? > > I guess because the window is assumed to have two fringes. But frame-fringe-width returns the sum of them both, doesn't it? > > > Probably (/ column (frame-char-width)) can be too large if you are > > > unlucky. > > > > Sorry, I don't follow. Can you elaborate on when this could happen? > > Note that in the shr-use-fonts -> t case, COLUMN is in pixels. > > (/ column (frame-char-width)) > > is IMO an estimated value of "real" columns to advance. No, it's the value in pixels expressed in frame's canonical character width. > But if there are many wide characters, this can be too large. No, it cannot. It doesn't depend in the width of individual characters, sine it doesn't count real columns. It's just a coordinate. > I think you looked at the wrong `if' branch...? What I changed was to > use `end-of-visual-line', which I hope is more accurate than > > (vertical-motion (cons (/ column (frame-char-width)) 0)) > > for finding the right point for breaking the line. Maybe, it sounds like I need to take a better look. Thanks. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-09 2:38 ` Eli Zaretskii @ 2015-07-09 11:01 ` Michael Heerdegen 2015-07-09 14:43 ` Eli Zaretskii 2015-07-09 15:34 ` Eli Zaretskii 1 sibling, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-07-09 11:01 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21012 Eli Zaretskii <eliz@gnu.org> writes: > > > Do you understand why the value of frame-fringe-width is multiplied by > > > 2? > > > > I guess because the window is assumed to have two fringes. > > But frame-fringe-width returns the sum of them both, doesn't it? Mmh, then it just doesn't make sense. > > Note that in the shr-use-fonts -> t case, COLUMN is in pixels. > > > > (/ column (frame-char-width)) > > > > is IMO an estimated value of "real" columns to advance. > > No, it's the value in pixels expressed in frame's canonical character > width. That doesn't sound right to me. COLUMN is in pixels, (frame-char-width) is in pixels, too. The quotient can't be in pixels. Its unit is just a number (of characters). Note that shr-vertical-motion doesn't receive a column number as argument, despite its name. Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-09 11:01 ` Michael Heerdegen @ 2015-07-09 14:43 ` Eli Zaretskii 2015-07-09 19:42 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Eli Zaretskii @ 2015-07-09 14:43 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: 21012@debbugs.gnu.org > Date: Thu, 09 Jul 2015 13:01:58 +0200 > > > > Note that in the shr-use-fonts -> t case, COLUMN is in pixels. > > > > > > (/ column (frame-char-width)) > > > > > > is IMO an estimated value of "real" columns to advance. > > > > No, it's the value in pixels expressed in frame's canonical character > > width. > > That doesn't sound right to me. COLUMN is in pixels, (frame-char-width) > is in pixels, too. The quotient can't be in pixels. Its unit is just a > number (of characters). Sorry, my wording must have been confusing. What I meant is that the above converts pixel coordinates into units of the frame's canonical characters. The important part is that this value doesn't change depending on the characters actually displayed, and in particular their width. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-09 14:43 ` Eli Zaretskii @ 2015-07-09 19:42 ` Michael Heerdegen 0 siblings, 0 replies; 38+ messages in thread From: Michael Heerdegen @ 2015-07-09 19:42 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21012 Eli Zaretskii <eliz@gnu.org> writes: > Sorry, my wording must have been confusing. What I meant is that the > above converts pixel coordinates into units of the frame's canonical > characters. The important part is that this value doesn't change > depending on the characters actually displayed, and in particular > their width. Ok, that's what I thought too. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-09 2:38 ` Eli Zaretskii 2015-07-09 11:01 ` Michael Heerdegen @ 2015-07-09 15:34 ` Eli Zaretskii 2015-07-09 20:06 ` Michael Heerdegen 1 sibling, 1 reply; 38+ messages in thread From: Eli Zaretskii @ 2015-07-09 15:34 UTC (permalink / raw) To: michael_heerdegen; +Cc: 21012 > Date: Thu, 09 Jul 2015 05:38:29 +0300 > From: Eli Zaretskii <eliz@gnu.org> > Cc: 21012@debbugs.gnu.org > > > I think you looked at the wrong `if' branch...? What I changed was to > > use `end-of-visual-line', which I hope is more accurate than > > > > (vertical-motion (cons (/ column (frame-char-width)) 0)) > > > > for finding the right point for breaking the line. > > Maybe, it sounds like I need to take a better look. I did that now. There are a few subtle issues with this, as described below. Please try the patch at the end of this message, and see if it gives good results. You didn't provide a reproducible test case for the screenshots you show, so I used Yamaoka-san's recipe; the patch below fixes all the issues I saw with that recipe. First, regarding your suggestion above to use end-of-visual-line: this is only TRT when shr-width is nil, i.e. shr is filling text to the full window width. But shr-vertical-motion should also support the case when shr-width is some specific number of columns or pixels, in which case end-of-visual-line will put us at the wrong position. Now to the issues I found. First, the value of shr-internal-width was computed incorrectly: it used window-width, which, as you discovered, is more than the text area width, and it subtracted 2 columns from that value, whereas it really needs to subtract only 1 (since column numbers are zero-based). The 2nd issue was in shr-find-fill-point, where it deals with CJK characters using the kinsoku feature: (a) it preferred to look for a breakable point forward even when shr-width is nil, which produces continuation lines; and (b) it mistakenly allowed to break the line before a character that is forbidden by kinsoku to be at BOL. Yet another subtlety is only visible when you disable one of the fringes (or both): the calculation of shr-internal-width should in that case subtract one more column, to be reserved for the continuation glyph. The patch below fixes that as well, albeit slightly crudely (we sometimes lose a column); suggestions for how to do that better are welcome. Here's a patch that attempts at fixing all but the last of these issues; if it gives good results, I will install it. If not, please show a test case that reproduces whatever problems are left. Thanks. diff --git a/lisp/net/shr.el b/lisp/net/shr.el index 0ce77b9..f440abf 100644 --- a/lisp/net/shr.el +++ b/lisp/net/shr.el @@ -222,10 +222,29 @@ (defun shr-insert-document (dom) (if (not shr-use-fonts) shr-width (* shr-width (frame-char-width)))) + ;; We need to adjust the available + ;; width for when the user disables + ;; the fringes, which will cause the + ;; display engine usurp one column for + ;; the continuation glyph. (if (not shr-use-fonts) - (- (window-width) 2) - (- (window-pixel-width) - (* (frame-fringe-width) 2)))))) + (- (window-body-width) 1 + (if (and (null shr-width) + (or (zerop + (fringe-columns 'right)) + (zerop + (fringe-columns 'left)))) + 0 + 1)) + (- (window-body-width nil t) + (frame-char-width) + (if (and (null shr-width) + (or (zerop + (fringe-columns 'right)) + (zerop + (fringe-columns 'left)))) + (* (frame-char-width) 2) + 0)))))) (shr-descend dom) (shr-fill-lines start (point)) (shr-remove-trailing-whitespace start (point)) @@ -439,8 +458,18 @@ (defun shr-fill-text (text) (with-temp-buffer (let ((shr-indentation 0) (shr-start nil) - (shr-internal-width (- (window-pixel-width) - (* (frame-fringe-width) 2)))) + (shr-internal-width (- (window-body-width nil t) + (frame-char-width) + ;; Adjust the window width for when + ;; the user disables the fringes, + ;; which causes the display engine + ;; usurp one coplumn for the + ;; continuation glyph. + (if (and (null shr-width) + (or (zerop (fringe-columns 'right)) + (zerop (fringe-columns 'left)))) + (* (frame-char-width) 2) + 0)))) (shr-insert text) (buffer-string))))) @@ -620,7 +649,9 @@ (defun shr-find-fill-point (start) ;; There's no breakable point, so we give it up. (let (found) (goto-char bp) - (unless shr-kinsoku-shorten + ;; Don't overflow the window edge, even if + ;; shr-kinsoku-shorten is nil. + (unless (or shr-kinsoku-shorten (null shr-width)) (while (setq found (re-search-forward "\\(\\c>\\)\\| \\|\\c<\\|\\c|" (line-end-position) 'move))) @@ -632,9 +663,12 @@ (defun shr-find-fill-point (start) ;; Don't put kinsoku-bol characters at the beginning of a line, ;; or kinsoku-eol characters at the end of a line. (cond - (shr-kinsoku-shorten + ;; Don't overflow the window edge, even if shr-kinsoku-shorten + ;; is nil. + ((or shr-kinsoku-shorten (null shr-width)) (while (and (not (memq (preceding-char) (list ?\C-@ ?\n ? ))) - (shr-char-kinsoku-eol-p (preceding-char))) + (or (shr-char-kinsoku-eol-p (preceding-char)) + (shr-char-kinsoku-bol-p (following-char)))) (backward-char 1)) (when (setq failed (<= (point) start)) ;; There's no breakable point that doesn't violate kinsoku, ^ permalink raw reply related [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-09 15:34 ` Eli Zaretskii @ 2015-07-09 20:06 ` Michael Heerdegen 2015-07-10 6:03 ` Eli Zaretskii 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-07-09 20:06 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21012 Eli Zaretskii <eliz@gnu.org> writes: > First, regarding your suggestion above to use end-of-visual-line: this > is only TRT when shr-width is nil, i.e. shr is filling text to the > full window width. But shr-vertical-motion should also support the > case when shr-width is some specific number of columns or pixels, in > which case end-of-visual-line will put us at the wrong position. Ok, I kind of expected that. > Now to the issues I found. > > First, the value of shr-internal-width was computed incorrectly: it > used window-width, which, as you discovered, is more than the text > area width, and it subtracted 2 columns from that value, whereas it > really needs to subtract only 1 (since column numbers are zero-based). Good. > Here's a patch that attempts at fixing all but the last of these > issues; if it gives good results, I will install it. If not, please > show a test case that reproduces whatever problems are left. > > [...] Ok, thanks much so far. The problem described by me (from the screenshot) is however not completely fixed (recipe follows). What I originally posted seems to happen with any text, it is more likely to happen when you use positive text-scale-mode, and it still happens. If you want a precise recipe (though it's just a random example): emacs -Q, C-h C-a, click on the first link (named "Gnu Emacs") with mouse-2. In line 135, the last char is truncated. If you hit C-x C-+, you will find more and more partly visible chars the more you scale up. Using `end-of-visual-line' seemed to fix this, but we can't use it. Is `window-text-pixel-size' useful? Thanks, Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-09 20:06 ` Michael Heerdegen @ 2015-07-10 6:03 ` Eli Zaretskii 2015-07-10 12:55 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Eli Zaretskii @ 2015-07-10 6:03 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: 21012@debbugs.gnu.org > Date: Thu, 09 Jul 2015 22:06:16 +0200 > > The problem described by me (from the screenshot) is however not > completely fixed (recipe follows). > > What I originally posted seems to happen with any text, it is more > likely to happen when you use positive text-scale-mode, and it still > happens. > > If you want a precise recipe (though it's just a random example): emacs > -Q, C-h C-a, click on the first link (named "Gnu Emacs") with mouse-2. > In line 135, the last char is truncated. > > If you hit C-x C-+, you will find more and more partly visible chars the > more you scale up. > > Using `end-of-visual-line' seemed to fix this, but we can't use it. > Is `window-text-pixel-size' useful? The problem here is that shr assumes that the last character on a visual line is always fully visible, which is false when truncate-lines is non-nil. The last call to forward-char in shr-vertical-motion should _not_ be made when the character we ended up at is too wide to be fully visible at the end of a visual line. The problem is that functions we have that can tell if that happens require the relevant portion of the buffer to be displayed in some window, so I think shr.el cannot use them. If the result of what we have now is too annoying, the only way to solve it I could think of is to artificially reduce the available window width by one more column. This would lose us one column in some cases, and will not solve the issue completely (a _very_ wide character will still be partially visible), but it could be a stopgap. Better ideas are welcome. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-10 6:03 ` Eli Zaretskii @ 2015-07-10 12:55 ` Michael Heerdegen 2015-07-10 13:06 ` Eli Zaretskii 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-07-10 12:55 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21012 Eli Zaretskii <eliz@gnu.org> writes: > The problem is that functions we have that can tell if that happens > require the relevant portion of the buffer to be displayed in some > window, so I think shr.el cannot use them. FWIW, the whole thing is already displayed, try (advice-add 'shr-fill-line :after (lambda (&rest _) (sit-for .5))) and let eww display some page. For a very large page, you can see a short flickering even without that advice when rendering. Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-10 12:55 ` Michael Heerdegen @ 2015-07-10 13:06 ` Eli Zaretskii 2015-07-10 14:16 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Eli Zaretskii @ 2015-07-10 13:06 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: 21012@debbugs.gnu.org > Date: Fri, 10 Jul 2015 14:55:01 +0200 > > Eli Zaretskii <eliz@gnu.org> writes: > > > The problem is that functions we have that can tell if that happens > > require the relevant portion of the buffer to be displayed in some > > window, so I think shr.el cannot use them. > > FWIW, the whole thing is already displayed Then please try this change and see if it gives good results: --- lisp/net/shr.el~0 2015-05-10 07:23:55 +0300 +++ lisp/net/shr.el 2015-07-09 19:24:11 +0300 @@ -549,20 +578,42 @@ (shr-fill-line))) (goto-char (point-max))))) -(defun shr-vertical-motion (column) +(defun shr-vertical-motion (column win-width) (if (not shr-use-fonts) (move-to-column column) (unless (eolp) (forward-char 1)) - (vertical-motion (cons (/ column (frame-char-width)) 0)) - (unless (eolp) - (forward-char 1)))) + (let ((orig-y (cdr (nth 2 (posn-at-point))))) + (vertical-motion (cons (/ column (frame-char-width)) 0)) + ;; If vertical-motion puts us on the next screen line, back up. + ;; This can happen when the character at the goal column is too + ;; wide to fit on the line. + (if (> (cdr (nth 2 (posn-at-point))) orig-y) + (backward-char 1)) + (unless (eolp) + (let* ((posn (posn-at-point)) + (ch-x (car (nth 2 posn))) + (ch-width (car (nth 9 posn))) + (ch-pos (nth 1 posn))) + (if (and (natnump ch-pos) + (<= (+ ch-x ch-width) win-width)) + (forward-char 1))))))) (defun shr-fill-line () (let ((shr-indentation (get-text-property (point) 'shr-indentation)) (continuation (get-text-property (point) 'shr-continuation-indentation)) - start) + start win-width) + (when shr-use-fonts + (setq win-width (window-body-width nil t)) + ;; When we are filling to the window width, and the user + ;; disabled the fringes, an additional column is reserved for + ;; the continuation glyph, so we need to adjust the effective + ;; window-width for that. + (if (and (null shr-width) + (or (zerop (fringe-columns 'left)) + (zerop (fringe-columns 'right)))) + (setq win-width (- win-width (frame-char-width))))) (put-text-property (point) (1+ (point)) 'shr-indentation nil) (let ((face (get-text-property (point) 'face)) (background-start (point))) @@ -572,7 +623,7 @@ `,(shr-face-background face)))) (setq start (point)) (setq shr-indentation (or continuation shr-indentation)) - (shr-vertical-motion shr-internal-width) + (shr-vertical-motion shr-internal-width win-width) (when (looking-at " $") (delete-region (point) (line-end-position))) (while (not (eolp)) @@ -597,7 +648,7 @@ (put-text-property background-start (point) 'face `,(shr-face-background face)))) (setq start (point)) - (shr-vertical-motion shr-internal-width) + (shr-vertical-motion shr-internal-width win-width) (when (looking-at " $") (delete-region (point) (line-end-position)))))) ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-10 13:06 ` Eli Zaretskii @ 2015-07-10 14:16 ` Michael Heerdegen 2015-07-10 14:43 ` Eli Zaretskii 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-07-10 14:16 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21012 Eli Zaretskii <eliz@gnu.org> writes: > > FWIW, the whole thing is already displayed > > Then please try this change and see if it gives good results: > > --- lisp/net/shr.el~0 2015-05-10 07:23:55 +0300 > +++ lisp/net/shr.el 2015-07-09 19:24:11 +0300 Unfortunately it doesn't work. Rendering now fails with an error: Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil) >(nil nil) (if (> (cdr (nth 2 (posn-at-point))) orig-y) (backward-char 1)) (let ((orig-y (cdr (nth 2 (posn-at-point))))) (vertical-motion (cons (/ column (frame-char-width)) 0)) (if (> (cdr (nth 2 (posn-at-point))) orig-y) (backward-char 1)) (if (eolp) nil (let* ((posn (posn-at-point)) (ch-x (car (nth 2 posn))) (ch-width (car (nth 9 posn))) (ch-pos (nth 1 posn))) (if (and (natnump ch-pos) (<= (+ ch-x ch-width) win-width)) (forward-char 1))))) (if (not shr-use-fonts) (move-to-column column) (if (eolp) nil (forward-char 1)) (let ((orig-y (cdr (nth 2 (posn-at-point))))) (vertical-motion (cons (/ column (frame-char-width)) 0)) (if (> (cdr (nth 2 (posn-at-point))) orig-y) (backward-char 1)) (if (eolp) nil (let* ((posn (posn-at-point)) (ch-x (car (nth 2 posn))) (ch-width (car (nth 9 posn))) (ch-pos (nth 1 posn))) (if (and (natnump ch-pos) (<= (+ ch-x ch-width) win-width)) (forward-char 1)))))) shr-vertical-motion(1250 1247) [...] shr-fill-line() because `posn-at-point' returns nil as soon as point moves past `window-end'. I can see that with the (advice-add 'shr-fill-line :after (lambda (&rest _) (sit-for .5))) advice active. Obviously it's not as simple as I thought, not the whole buffer is "already displayed". My bad. Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-10 14:16 ` Michael Heerdegen @ 2015-07-10 14:43 ` Eli Zaretskii 2015-07-10 18:04 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Eli Zaretskii @ 2015-07-10 14:43 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: 21012@debbugs.gnu.org > Date: Fri, 10 Jul 2015 16:16:00 +0200 > > Eli Zaretskii <eliz@gnu.org> writes: > > > > FWIW, the whole thing is already displayed > > > > Then please try this change and see if it gives good results: > > > > --- lisp/net/shr.el~0 2015-05-10 07:23:55 +0300 > > +++ lisp/net/shr.el 2015-07-09 19:24:11 +0300 > > Unfortunately it doesn't work. Rendering now fails with an error: > > Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil) That's exactly what I expected. > because `posn-at-point' returns nil as soon as point moves past > `window-end'. Yes, that's what I meant by "the relevant portion of the buffer [has] to be displayed in some window". If point is not visible, posn-at-point will return nil. It is easy to make the code fail gracefully in those cases, but that would mean the problems this is trying to solve will be solved only in some part of the buffer. Which I think is worse than leaving them as they are now. > Obviously it's not as simple as I thought, not the whole buffer is > "already displayed". My bad. So how about trying to take one more column from the width available for filling? Does that produce reasonable results? ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-10 14:43 ` Eli Zaretskii @ 2015-07-10 18:04 ` Michael Heerdegen 2015-07-10 18:45 ` Eli Zaretskii 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-07-10 18:04 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21012 Eli Zaretskii <eliz@gnu.org> writes: > So how about trying to take one more column from the width available > for filling? Does that produce reasonable results? I'll try that when I'm convinced that we can't do better. Can't we find the position of the last completely visible char of a line with --8<---------------cut here---------------start------------->8--- (save-excursion (let ((truncate-lines nil)) (when (line-move-visual +1 t) (backward-char 1)) (point))) --8<---------------cut here---------------end--------------->8--- Is that sane? Can we use that? Seems to work. Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-10 18:04 ` Michael Heerdegen @ 2015-07-10 18:45 ` Eli Zaretskii 2015-07-10 19:19 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Eli Zaretskii @ 2015-07-10 18:45 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: 21012@debbugs.gnu.org > Date: Fri, 10 Jul 2015 20:04:43 +0200 > > Can't we find the position of the last completely visible char of a > line with > > --8<---------------cut here---------------start------------->8--- > (save-excursion > (let ((truncate-lines nil)) > (when (line-move-visual +1 t) > (backward-char 1)) > (point))) > --8<---------------cut here---------------end--------------->8--- I'd prefer to exchange ideas rather than code that I need to second guess. So what's the idea behind this? Is the idea to turn off truncate-lines and thus avoid partially visible characters at end of line? If so, how do you find the last visible character? Doing what you suggest above won't work with bidirectional text, where (backward-char 1) from the leftmost character of a visual line doesn't necessarily put you on the last character of the previous visual line. I think you need to let-bind visual-order-cursor-movement to t and invoke left-char instead. Next, line-move-visual will only move to the leftmost character of the next line if you are already on the leftmost character of the current line, so you'd need to make that happen first. Not hard to do, but should be part of the code, or maybe I'm missing something. Finally, portions of line-move-visual and its subroutines only work when the line is visible in some window, so won't that again hit the same limitation of posn-at-point? Thanks. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-10 18:45 ` Eli Zaretskii @ 2015-07-10 19:19 ` Michael Heerdegen 2015-07-10 19:31 ` Eli Zaretskii 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-07-10 19:19 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21012 Eli Zaretskii <eliz@gnu.org> writes: > > Can't we find the position of the last completely visible char of a > > line with > > > > --8<---------------cut here---------------start------------->8--- > > (save-excursion > > (let ((truncate-lines nil)) > > (when (line-move-visual +1 t) > > (backward-char 1)) > > (point))) > > --8<---------------cut here---------------end--------------->8--- > > I'd prefer to exchange ideas rather than code that I need to second > guess. So what's the idea behind this? > > Is the idea to turn off truncate-lines and thus avoid partially > visible characters at end of line? I thought this is obvious. > If so, how do you find the last visible character? Doing what you > suggest above won't work with bidirectional text, where (backward-char > 1) from the leftmost character of a visual line doesn't necessarily > put you on the last character of the previous visual line. I think > you need to let-bind visual-order-cursor-movement to t and invoke > left-char instead. Ok. > Next, line-move-visual will only move to the leftmost character of the > next line if you are already on the leftmost character of the current > line, so you'd need to make that happen first. Not hard to do, but > should be part of the code, or maybe I'm missing something. I missed that because I always have goal-column -> 0. > Finally, portions of line-move-visual and its subroutines only work > when the line is visible in some window, so won't that again hit the > same limitation of posn-at-point? I don't know. That's why the last sentences in my message were questions. But are you sure that `vertical-motion' does not hit that limitation? For example, AFAIK `vertical-motion' stops at the end of a visual line if the first arg is a cons cell with COLS larger than available cols in the line. How does that work when that line is not displayed somewhere? Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-10 19:19 ` Michael Heerdegen @ 2015-07-10 19:31 ` Eli Zaretskii 2015-07-11 12:02 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Eli Zaretskii @ 2015-07-10 19:31 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: 21012@debbugs.gnu.org > Date: Fri, 10 Jul 2015 21:19:18 +0200 > > > Finally, portions of line-move-visual and its subroutines only work > > when the line is visible in some window, so won't that again hit the > > same limitation of posn-at-point? > > I don't know. That's why the last sentences in my message were > questions. > > But are you sure that `vertical-motion' does not hit that limitation? vertical-motion needs the buffer displayed in some window, but it does not need the text it traversed to be visible in that window. Thus, it has fewer limitations that posn-at-point and pos-visible-in-window-p, which are called by line-move-visual. > For example, AFAIK `vertical-motion' stops at the end of a visual line > if the first arg is a cons cell with COLS larger than available cols in > the line. Yes. > How does that work when that line is not displayed somewhere? It simulates display, i.e. runs the same code as would be used to lay out the text in question, but without actually displaying it. It should be possible, in principle, to write something similar to posn-at-point that would not depend on the text being visible, or rewrite posn-at-point to free it from this limitation, but we don't have such a function at this time. Maybe we should. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-10 19:31 ` Eli Zaretskii @ 2015-07-11 12:02 ` Michael Heerdegen 2015-07-11 13:45 ` Eli Zaretskii 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-07-11 12:02 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21012 [-- Attachment #1: Type: text/plain, Size: 686 bytes --] Eli Zaretskii <eliz@gnu.org> writes: > It should be possible, in principle, to write something similar to > posn-at-point that would not depend on the text being visible, or > rewrite posn-at-point to free it from this limitation, but we don't > have such a function at this time. Maybe we should. Of course would that be useful, not only for this issue. But I guess it would be a lot of work. I try now what you suggested, subtracting one more column width when calculating `shr-internal-width'. I.e., I subtract one more `frame-char-width' now. Looks good so far. I'll use it for some time and tell you what I think. So, here is what I'm testing, based on your first patch: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: shr-02.patch --] [-- Type: text/x-diff, Size: 4012 bytes --] --- a/lisp/net/shr.el +++ b/lisp/net/shr.el @@ -222,10 +222,29 @@ DOM should be a parse tree as generated by (if (not shr-use-fonts) shr-width (* shr-width (frame-char-width)))) + ;; We need to adjust the available + ;; width for when the user disables + ;; the fringes, which will cause the + ;; display engine usurp one column for + ;; the continuation glyph. (if (not shr-use-fonts) - (- (window-width) 2) - (- (window-pixel-width) - (* (frame-fringe-width) 2)))))) + (- (window-body-width) 1 + (if (and (null shr-width) + (or (zerop + (fringe-columns 'right)) + (zerop + (fringe-columns 'left)))) + 0 + 1)) + (- (window-body-width nil t) + (* 2 (frame-char-width)) + (if (and (null shr-width) + (or (zerop + (fringe-columns 'right)) + (zerop + (fringe-columns 'left)))) + (* (frame-char-width) 2) + 0)))))) (shr-descend dom) (shr-fill-lines start (point)) (shr-remove-trailing-whitespace start (point)) @@ -439,8 +458,18 @@ size, and full-buffer size." (with-temp-buffer (let ((shr-indentation 0) (shr-start nil) - (shr-internal-width (- (window-pixel-width) - (* (frame-fringe-width) 2)))) + (shr-internal-width (- (window-body-width nil t) + (* 2 (frame-char-width)) + ;; Adjust the window width for when + ;; the user disables the fringes, + ;; which causes the display engine + ;; usurp one coplumn for the + ;; continuation glyph. + (if (and (null shr-width) + (or (zerop (fringe-columns 'right)) + (zerop (fringe-columns 'left)))) + (* (frame-char-width) 2) + 0)))) (shr-insert text) (buffer-string))))) @@ -620,7 +649,9 @@ size, and full-buffer size." ;; There's no breakable point, so we give it up. (let (found) (goto-char bp) - (unless shr-kinsoku-shorten + ;; Don't overflow the window edge, even if + ;; shr-kinsoku-shorten is nil. + (unless (or shr-kinsoku-shorten (null shr-width)) (while (setq found (re-search-forward "\\(\\c>\\)\\| \\|\\c<\\|\\c|" (line-end-position) 'move))) @@ -632,9 +663,12 @@ size, and full-buffer size." ;; Don't put kinsoku-bol characters at the beginning of a line, ;; or kinsoku-eol characters at the end of a line. (cond - (shr-kinsoku-shorten + ;; Don't overflow the window edge, even if shr-kinsoku-shorten + ;; is nil. + ((or shr-kinsoku-shorten (null shr-width)) (while (and (not (memq (preceding-char) (list ?\C-@ ?\n ? ))) - (shr-char-kinsoku-eol-p (preceding-char))) + (or (shr-char-kinsoku-eol-p (preceding-char)) + (shr-char-kinsoku-bol-p (following-char)))) (backward-char 1)) (when (setq failed (<= (point) start)) ;; There's no breakable point that doesn't violate kinsoku, [-- Attachment #3: Type: text/plain, Size: 20 bytes --] Thanks, Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-11 12:02 ` Michael Heerdegen @ 2015-07-11 13:45 ` Eli Zaretskii 2015-07-20 16:33 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Eli Zaretskii @ 2015-07-11 13:45 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: 21012@debbugs.gnu.org > Date: Sat, 11 Jul 2015 14:02:42 +0200 > > > It should be possible, in principle, to write something similar to > > posn-at-point that would not depend on the text being visible, or > > rewrite posn-at-point to free it from this limitation, but we don't > > have such a function at this time. Maybe we should. > > Of course would that be useful, not only for this issue. But I guess it > would be a lot of work. Actually, it's not a lot of work. Patches welcome, and I will look into doing that if no one does until I have enough free time (which currently could take a while). > I try now what you suggested, subtracting one more column width when > calculating `shr-internal-width'. I.e., I subtract one more > `frame-char-width' now. Looks good so far. I'll use it for some time > and tell you what I think. > > So, here is what I'm testing, based on your first patch: Thanks. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-11 13:45 ` Eli Zaretskii @ 2015-07-20 16:33 ` Michael Heerdegen 2015-07-20 16:34 ` Eli Zaretskii 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-07-20 16:33 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21012 Eli Zaretskii <eliz@gnu.org> writes: > > So, here is what I'm testing, based on your first patch: > > Thanks. Used it quite a while now, and I think it's fine. Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-20 16:33 ` Michael Heerdegen @ 2015-07-20 16:34 ` Eli Zaretskii 2015-07-21 18:49 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Eli Zaretskii @ 2015-07-20 16:34 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: 21012@debbugs.gnu.org > Date: Mon, 20 Jul 2015 18:33:20 +0200 > > Eli Zaretskii <eliz@gnu.org> writes: > > > > So, here is what I'm testing, based on your first patch: > > > > Thanks. > > Used it quite a while now, and I think it's fine. Great, then please install it. Thanks. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-20 16:34 ` Eli Zaretskii @ 2015-07-21 18:49 ` Michael Heerdegen 2015-09-25 4:00 ` Katsumi Yamaoka 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-07-21 18:49 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21012 Eli Zaretskii <eliz@gnu.org> writes: > Great, then please install it. Ok, will do soon, thanks! Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-21 18:49 ` Michael Heerdegen @ 2015-09-25 4:00 ` Katsumi Yamaoka 2015-09-25 14:45 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Katsumi Yamaoka @ 2015-09-25 4:00 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 21012 On Tue, 21 Jul 2015 20:49:36 +0200, Michael Heerdegen wrote: > Eli Zaretskii <eliz@gnu.org> writes: >> Great, then please install it. > Ok, will do soon, thanks! The shr-02.patch[1] perfectly fixes the problem at least for Japanese html mails[2]. Could you please install it? I've been using the patched one for more than a month. Thanks. [1] <http://thread.gmane.org/gmane.emacs.bugs/104835/focus=104909> [2] <http://thread.gmane.org/gmane.emacs.devel/187712/focus=187717> ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-09-25 4:00 ` Katsumi Yamaoka @ 2015-09-25 14:45 ` Michael Heerdegen 2015-09-28 21:30 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-09-25 14:45 UTC (permalink / raw) To: Katsumi Yamaoka; +Cc: 21012 Katsumi Yamaoka <yamaoka@jpl.org> writes: > > Ok, will do soon, thanks! > > The shr-02.patch[1] perfectly fixes the problem at least for > Japanese html mails[2]. Could you please install it? > I've been using the patched one for more than a month. Fine. I'll make it my priority for today to finish reading CONTRIBUTE and make the commit. Sorry for the delay. Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-09-25 14:45 ` Michael Heerdegen @ 2015-09-28 21:30 ` Michael Heerdegen 2015-09-29 5:37 ` Eli Zaretskii 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-09-28 21:30 UTC (permalink / raw) To: Katsumi Yamaoka; +Cc: 21012 Michael Heerdegen <michael_heerdegen@web.de> writes: > Fine. I'll make it my priority for today to finish reading CONTRIBUTE > and make the commit. Sorry for the delay. Making some progress here... Eli, would it be ok when I create a temporary test branch for my first commit forked from master? We can delete it after one day or so. Of course I'm not going to merge this branch into master but make a separate equal commit there. Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-09-28 21:30 ` Michael Heerdegen @ 2015-09-29 5:37 ` Eli Zaretskii 2015-10-03 8:08 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Eli Zaretskii @ 2015-09-29 5:37 UTC (permalink / raw) To: Michael Heerdegen; +Cc: yamaoka, 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Date: Mon, 28 Sep 2015 23:30:34 +0200 > Cc: 21012@debbugs.gnu.org > > Michael Heerdegen <michael_heerdegen@web.de> writes: > > > Fine. I'll make it my priority for today to finish reading CONTRIBUTE > > and make the commit. Sorry for the delay. > > Making some progress here... > > Eli, would it be ok when I create a temporary test branch for my first > commit forked from master? We can delete it after one day or so. Of > course I'm not going to merge this branch into master but make a > separate equal commit there. Fine with me, please go ahead. Thanks. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-09-29 5:37 ` Eli Zaretskii @ 2015-10-03 8:08 ` Michael Heerdegen 2015-10-03 9:42 ` Eli Zaretskii 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-10-03 8:08 UTC (permalink / raw) To: Eli Zaretskii; +Cc: yamaoka, 21012 [-- Attachment #1: Type: text/plain, Size: 228 bytes --] Eli Zaretskii <eliz@gnu.org> writes: > Fine with me, please go ahead. I thought it would be easier to send you the contents of commit buffer in magit - it includes even more information than git format-patch -1. Here it is: [-- Attachment #2: the-commit.txt --] [-- Type: text/plain, Size: 4745 bytes --] master d72b05282033821884cfaef59f278a2bf6af33b6 Author: Eli Zaretskii <eliz@gnu.org> AuthorDate: Thu Jul 9 05:38:29 2015 +0300 Commit: Michael Heerdegen <michael_heerdegen@web.de> CommitDate: Sat Oct 3 09:43:03 2015 +0200 Parent: e160525 Adapt to new prettify-symbols-unprettify-at-point default Merged: master Containing: master Follows: emacs-24.5-rc3-fixed (6094) shr: fix too long lines in rendered buffers (Bug#21012) * lisp/net/shr.el (shr-insert-document, shr-fill-text): Correct calculation of available width. (shr-find-fill-point): Don't overflow window edge if shr-kinsoku-shorten is nil. 1 file changed, 42 insertions(+), 8 deletions(-) lisp/net/shr.el | 50 ++++++++++++++++++++++++++++++++++++++++++-------- modified lisp/net/shr.el @@ -222,10 +222,29 @@ DOM should be a parse tree as generated by (if (not shr-use-fonts) shr-width (* shr-width (frame-char-width)))) + ;; We need to adjust the available + ;; width for when the user disables + ;; the fringes, which will cause the + ;; display engine usurp one column for + ;; the continuation glyph. (if (not shr-use-fonts) - (- (window-width) 2) - (- (window-pixel-width) - (* (frame-fringe-width) 2)))))) + (- (window-body-width) 1 + (if (and (null shr-width) + (or (zerop + (fringe-columns 'right)) + (zerop + (fringe-columns 'left)))) + 0 + 1)) + (- (window-body-width nil t) + (* 2 (frame-char-width)) + (if (and (null shr-width) + (or (zerop + (fringe-columns 'right)) + (zerop + (fringe-columns 'left)))) + (* (frame-char-width) 2) + 0)))))) (shr-descend dom) (shr-fill-lines start (point)) (shr-remove-trailing-whitespace start (point)) @@ -439,8 +458,18 @@ size, and full-buffer size." (with-temp-buffer (let ((shr-indentation 0) (shr-start nil) - (shr-internal-width (- (window-pixel-width) - (* (frame-fringe-width) 2)))) + (shr-internal-width (- (window-body-width nil t) + (* 2 (frame-char-width)) + ;; Adjust the window width for when + ;; the user disables the fringes, + ;; which causes the display engine + ;; usurp one coplumn for the + ;; continuation glyph. + (if (and (null shr-width) + (or (zerop (fringe-columns 'right)) + (zerop (fringe-columns 'left)))) + (* (frame-char-width) 2) + 0)))) (shr-insert text) (buffer-string))))) @@ -620,7 +649,9 @@ size, and full-buffer size." ;; There's no breakable point, so we give it up. (let (found) (goto-char bp) - (unless shr-kinsoku-shorten + ;; Don't overflow the window edge, even if + ;; shr-kinsoku-shorten is nil. + (unless (or shr-kinsoku-shorten (null shr-width)) (while (setq found (re-search-forward "\\(\\c>\\)\\| \\|\\c<\\|\\c|" (line-end-position) 'move))) @@ -632,9 +663,12 @@ size, and full-buffer size." ;; Don't put kinsoku-bol characters at the beginning of a line, ;; or kinsoku-eol characters at the end of a line. (cond - (shr-kinsoku-shorten + ;; Don't overflow the window edge, even if shr-kinsoku-shorten + ;; is nil. + ((or shr-kinsoku-shorten (null shr-width)) (while (and (not (memq (preceding-char) (list ?\C-@ ?\n ? ))) - (shr-char-kinsoku-eol-p (preceding-char))) + (or (shr-char-kinsoku-eol-p (preceding-char)) + (shr-char-kinsoku-bol-p (following-char)))) (backward-char 1)) (when (setq failed (<= (point) start)) ;; There's no breakable point that doesn't violate kinsoku, [-- Attachment #3: Type: text/plain, Size: 442 bytes --] Does that all make sense and play by the rules (commit message)? I specified you as the author (since all but some chars is from you), and specified the date you had sent the patch as commit --date (aka "author date"). Oh, and I don't know anything about `shr-kinsoku-shorten', I hope what I wrote makes sense. If you are agreed with everything, I plan to pull --rebase, and push. Thanks for your review in advance. Regards, Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-10-03 8:08 ` Michael Heerdegen @ 2015-10-03 9:42 ` Eli Zaretskii 2015-10-03 12:41 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Eli Zaretskii @ 2015-10-03 9:42 UTC (permalink / raw) To: Michael Heerdegen; +Cc: yamaoka, 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: yamaoka@jpl.org, 21012@debbugs.gnu.org > Date: Sat, 03 Oct 2015 10:08:42 +0200 > > Eli Zaretskii <eliz@gnu.org> writes: > > > Fine with me, please go ahead. > > I thought it would be easier to send you the contents of commit buffer > in magit - it includes even more information than git format-patch -1. > > Here it is: LGTM, thanks. > Does that all make sense and play by the rules (commit message)? Yes. > I specified you as the author (since all but some chars is from you), That's not entirely accurate: some code was from you, AFAIR. So maybe mention your name in the log message. > and specified the date you had sent the patch as commit --date (aka > "author date"). Sorry, I don't know what that means, for when the commit will be made. Is this some additional metadata, something that is usually absent in our commits? If so, it's fine with me. > Oh, and I don't know anything about `shr-kinsoku-shorten', I hope what I > wrote makes sense. It does. > If you are agreed with everything, I plan to pull --rebase, and push. Please go ahead. I understand that you will rebase and push to master, not to the emacs-24 branch, right? Thanks. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-10-03 9:42 ` Eli Zaretskii @ 2015-10-03 12:41 ` Michael Heerdegen 2015-10-04 6:31 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-10-03 12:41 UTC (permalink / raw) To: Eli Zaretskii; +Cc: yamaoka, 21012 Eli Zaretskii <eliz@gnu.org> writes: > Sorry, I don't know what that means, for when the commit will be > made. Is this some additional metadata, something that is usually > absent in our commits? If so, it's fine with me. Yip - see CONTRIBUTE, which told me I should do that ;-) In git, each commit has a committer id + date, and an author id + date, which default to the first pair when you don't specify them. > Please go ahead. I understand that you will rebase and push to > master, not to the emacs-24 branch, right? Yes, that's what I meant. Thanks for reviewing! Pushing will be the first thing I do tomorrow. Regards, Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-10-03 12:41 ` Michael Heerdegen @ 2015-10-04 6:31 ` Michael Heerdegen 2015-10-04 7:09 ` Eli Zaretskii 2015-10-04 7:11 ` Michael Heerdegen 0 siblings, 2 replies; 38+ messages in thread From: Michael Heerdegen @ 2015-10-04 6:31 UTC (permalink / raw) To: Eli Zaretskii; +Cc: yamaoka, 21012 Michael Heerdegen <michael_heerdegen@web.de> writes: > Pushing will be the first thing I do tomorrow. Tried to push; getting this: > git push Counting objects: 5, done. Writing objects: 100% (5/5), 1.17 KiB | 0 bytes/s, done. Total 5 (delta 4), reused 0 (delta 0) error: unpack failed: unpack-objects abnormal exit To git://git.savannah.gnu.org/emacs.git ! [remote rejected] master -> master (n/a (unpacker error)) error: failed to push some refs to 'git://git.savannah.gnu.org/emacs.git' What's wrong here? FWIW I can commit to the Elpa repo without any problem. TIA, Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-10-04 6:31 ` Michael Heerdegen @ 2015-10-04 7:09 ` Eli Zaretskii 2015-10-04 7:11 ` Michael Heerdegen 1 sibling, 0 replies; 38+ messages in thread From: Eli Zaretskii @ 2015-10-04 7:09 UTC (permalink / raw) To: Michael Heerdegen; +Cc: yamaoka, 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: yamaoka@jpl.org, 21012@debbugs.gnu.org > Date: Sun, 04 Oct 2015 08:31:51 +0200 > > > git push > Counting objects: 5, done. > Writing objects: 100% (5/5), 1.17 KiB | 0 bytes/s, done. > Total 5 (delta 4), reused 0 (delta 0) > error: unpack failed: unpack-objects abnormal exit > To git://git.savannah.gnu.org/emacs.git > ! [remote rejected] master -> master (n/a (unpacker error)) > error: failed to push some refs to 'git://git.savannah.gnu.org/emacs.git' > > What's wrong here? FWIW I can commit to the Elpa repo without any > problem. No idea, sorry. It never happened to me. Perhaps some Git expert could help. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-10-04 6:31 ` Michael Heerdegen 2015-10-04 7:09 ` Eli Zaretskii @ 2015-10-04 7:11 ` Michael Heerdegen 2015-10-04 7:39 ` Michael Heerdegen 1 sibling, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-10-04 7:11 UTC (permalink / raw) To: Eli Zaretskii; +Cc: yamaoka, 21012 Michael Heerdegen <michael_heerdegen@web.de> writes: > To git://git.savannah.gnu.org/emacs.git > ! [remote rejected] master -> master (n/a (unpacker error)) > error: failed to push some refs to 'git://git.savannah.gnu.org/emacs.git' Searching the net seems to suggest that I need to use a different remote url to be able to communicate via ssh. I read over https://savannah.gnu.org/projects/emacs/ and http://www.emacswiki.org/emacs/GitForEmacsDevs, but that doesn't seem to cover that question. Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-10-04 7:11 ` Michael Heerdegen @ 2015-10-04 7:39 ` Michael Heerdegen 2015-10-04 8:49 ` Eli Zaretskii 0 siblings, 1 reply; 38+ messages in thread From: Michael Heerdegen @ 2015-10-04 7:39 UTC (permalink / raw) To: Eli Zaretskii; +Cc: yamaoka, 21012 Michael Heerdegen <michael_heerdegen@web.de> writes: > Searching the net seems to suggest that I need to use a different remote > url to be able to communicate via ssh. > > I read over https://savannah.gnu.org/projects/emacs/ and > http://www.emacswiki.org/emacs/GitForEmacsDevs, but that doesn't seem to > cover that question. Found it in http://savannah.gnu.org/maintenance/SavannahServices/. After changing the remote url to mheerdegen@git.sv.gnu.org:/srv/git/emacs.git I could finally push. Hope it's alright. Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-10-04 7:39 ` Michael Heerdegen @ 2015-10-04 8:49 ` Eli Zaretskii 2015-10-04 10:18 ` Michael Heerdegen 0 siblings, 1 reply; 38+ messages in thread From: Eli Zaretskii @ 2015-10-04 8:49 UTC (permalink / raw) To: Michael Heerdegen; +Cc: yamaoka, 21012 > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: yamaoka@jpl.org, 21012@debbugs.gnu.org > Date: Sun, 04 Oct 2015 09:39:18 +0200 > > Michael Heerdegen <michael_heerdegen@web.de> writes: > > > Searching the net seems to suggest that I need to use a different remote > > url to be able to communicate via ssh. > > > > I read over https://savannah.gnu.org/projects/emacs/ and > > http://www.emacswiki.org/emacs/GitForEmacsDevs, but that doesn't seem to > > cover that question. > > Found it in http://savannah.gnu.org/maintenance/SavannahServices/. > After changing the remote url to > > mheerdegen@git.sv.gnu.org:/srv/git/emacs.git Right, the one you used was for read-only access. Sorry for not paying attention to that part. > I could finally push. Hope it's alright. Looks good here, thanks. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-10-04 8:49 ` Eli Zaretskii @ 2015-10-04 10:18 ` Michael Heerdegen 0 siblings, 0 replies; 38+ messages in thread From: Michael Heerdegen @ 2015-10-04 10:18 UTC (permalink / raw) To: Eli Zaretskii; +Cc: yamaoka, 21012 Eli Zaretskii <eliz@gnu.org> writes: > Looks good here, thanks. Great. Then I think I'll close this bug tomorrow. Thanks all for helping. Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible 2015-07-08 20:03 ` Eli Zaretskii 2015-07-08 20:24 ` Michael Heerdegen @ 2015-07-08 20:31 ` Michael Heerdegen 1 sibling, 0 replies; 38+ messages in thread From: Michael Heerdegen @ 2015-07-08 20:31 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21012 Eli Zaretskii <eliz@gnu.org> writes: > I believe the problem is that the code determines whether the line > should be wrapped based on this test: > > (shr-vertical-motion shr-internal-width) > (when (looking-at " $") <<<<<<<<<<<<<<<<<<<<<<<<< > (delete-region (point) (line-end-position))) > I think no. AFAIU (shr-vertical-motion shr-internal-width) goes to the position where the line should be broken at the latest. I think the code you cited just deletes any whitespace before breaking the line. Michael. ^ permalink raw reply [flat|nested] 38+ messages in thread
* bug#21012: Close 2015-07-08 18:10 bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible Michael Heerdegen 2015-07-08 20:03 ` Eli Zaretskii @ 2015-10-07 6:34 ` Michael Heerdegen 1 sibling, 0 replies; 38+ messages in thread From: Michael Heerdegen @ 2015-10-07 6:34 UTC (permalink / raw) To: 21012-done Closed via "shr: fix too long lines in rendered buffers (Bug#21012)". ^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2015-10-07 6:34 UTC | newest] Thread overview: 38+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-07-08 18:10 bug#21012: 25.0.50; eww: last char of a line sometimes not fully visible Michael Heerdegen 2015-07-08 20:03 ` Eli Zaretskii 2015-07-08 20:24 ` Michael Heerdegen 2015-07-09 2:38 ` Eli Zaretskii 2015-07-09 11:01 ` Michael Heerdegen 2015-07-09 14:43 ` Eli Zaretskii 2015-07-09 19:42 ` Michael Heerdegen 2015-07-09 15:34 ` Eli Zaretskii 2015-07-09 20:06 ` Michael Heerdegen 2015-07-10 6:03 ` Eli Zaretskii 2015-07-10 12:55 ` Michael Heerdegen 2015-07-10 13:06 ` Eli Zaretskii 2015-07-10 14:16 ` Michael Heerdegen 2015-07-10 14:43 ` Eli Zaretskii 2015-07-10 18:04 ` Michael Heerdegen 2015-07-10 18:45 ` Eli Zaretskii 2015-07-10 19:19 ` Michael Heerdegen 2015-07-10 19:31 ` Eli Zaretskii 2015-07-11 12:02 ` Michael Heerdegen 2015-07-11 13:45 ` Eli Zaretskii 2015-07-20 16:33 ` Michael Heerdegen 2015-07-20 16:34 ` Eli Zaretskii 2015-07-21 18:49 ` Michael Heerdegen 2015-09-25 4:00 ` Katsumi Yamaoka 2015-09-25 14:45 ` Michael Heerdegen 2015-09-28 21:30 ` Michael Heerdegen 2015-09-29 5:37 ` Eli Zaretskii 2015-10-03 8:08 ` Michael Heerdegen 2015-10-03 9:42 ` Eli Zaretskii 2015-10-03 12:41 ` Michael Heerdegen 2015-10-04 6:31 ` Michael Heerdegen 2015-10-04 7:09 ` Eli Zaretskii 2015-10-04 7:11 ` Michael Heerdegen 2015-10-04 7:39 ` Michael Heerdegen 2015-10-04 8:49 ` Eli Zaretskii 2015-10-04 10:18 ` Michael Heerdegen 2015-07-08 20:31 ` Michael Heerdegen 2015-10-07 6:34 ` bug#21012: Close Michael Heerdegen
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.