GNU bug report logs -
#35062
Patches: trivial cleanups
Previous Next
Reported by: Konstantin Kharlamov <hi-angel <at> yandex.ru>
Date: Sun, 31 Mar 2019 22:37:02 UTC
Severity: wishlist
Tags: fixed
Fixed in version 27.1
Done: Lars Ingebrigtsen <larsi <at> gnus.org>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 35062 in the body.
You can then email your comments to 35062 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 31 Mar 2019 22:37:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Konstantin Kharlamov <hi-angel <at> yandex.ru>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Sun, 31 Mar 2019 22:37:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
Per discussion on emacs-devel, this topic is created so that following
patches wouldn't create separate reports.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 31 Mar 2019 22:38:03 GMT)
Full text and
rfc822 format available.
Message #8 received at 35062 <at> debbugs.gnu.org (full text, mbox):
These are mostly fixes of some of LGTM warnings
https://lgtm.com/projects/g/emacs-mirror/emacs/alerts/?mode=tree
Except the second patch, where I initially wanted to fix one warning,
and as part of it I had to constify a variable to see that it is indeed
immutable. And then I figured I could search through the code and find
more similar places, where variables weren't marked as const. I like
this cleanup because it is simple and trivially testable (i.e. if it
compiles, then it's fine). FTR: there's still lots of opportunities for
constification, I just stopped at some point.
Konstantin Kharlamov (4):
Remove redundant comparison
constify a bit of xterm.c
min_cols/rows is always 0, don't check for it
don't compare unsigned to less-than-zero
src/gtkutil.c | 5 +----
src/pdumper.c | 2 --
src/xterm.c | 34 +++++++++++++++++-----------------
3 files changed, 18 insertions(+), 23 deletions(-)
--
2.21.0
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 31 Mar 2019 22:38:04 GMT)
Full text and
rfc822 format available.
Message #11 received at 35062 <at> debbugs.gnu.org (full text, mbox):
* src/xterm.c: due to preceding checks it is bound to be 0 ≤ alpha ≤ 1.
Fixes LGTM warning: comparison is always true because 0 <= alpha.
---
src/xterm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/xterm.c b/src/xterm.c
index f90d6713b02..e8f1e576a38 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -931,7 +931,7 @@ x_set_frame_alpha (struct frame *f)
return;
else if (alpha > 1.0)
alpha = 1.0;
- else if (0.0 <= alpha && alpha < alpha_min && alpha_min <= 1.0)
+ else if (alpha < alpha_min && alpha_min <= 1.0)
alpha = alpha_min;
opac = alpha * OPAQUE;
--
2.21.0
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 31 Mar 2019 22:38:04 GMT)
Full text and
rfc822 format available.
Message #14 received at 35062 <at> debbugs.gnu.org (full text, mbox):
* src/xterm.c: make code easier to follow by making explicit that some
variables are immutable
---
src/xterm.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/src/xterm.c b/src/xterm.c
index e8f1e576a38..8354ce00700 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -495,9 +495,9 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_surface_t *image,
cairo_fill_preserve (cr);
}
- int orig_image_width = cairo_image_surface_get_width (image);
+ const int orig_image_width = cairo_image_surface_get_width (image);
if (image_width == 0) image_width = orig_image_width;
- int orig_image_height = cairo_image_surface_get_height (image);
+ const int orig_image_height = cairo_image_surface_get_height (image);
if (image_height == 0) image_height = orig_image_height;
cairo_pattern_t *pattern = cairo_pattern_create_for_surface (image);
@@ -1006,7 +1006,7 @@ x_update_begin (struct frame *f)
if (FRAME_GTK_WIDGET (f))
{
GdkWindow *w = gtk_widget_get_window (FRAME_GTK_WIDGET (f));
- int scale = xg_get_scale (f);
+ const int scale = xg_get_scale (f);
width = scale * gdk_window_get_width (w);
height = scale * gdk_window_get_height (w);
}
@@ -1300,15 +1300,15 @@ x_clear_under_internal_border (struct frame *f)
{
if (FRAME_INTERNAL_BORDER_WIDTH (f) > 0)
{
- int border = FRAME_INTERNAL_BORDER_WIDTH (f);
- int width = FRAME_PIXEL_WIDTH (f);
- int height = FRAME_PIXEL_HEIGHT (f);
+ const int border = FRAME_INTERNAL_BORDER_WIDTH (f);
+ const int width = FRAME_PIXEL_WIDTH (f);
+ const int height = FRAME_PIXEL_HEIGHT (f);
#ifdef USE_GTK
- int margin = 0;
+ const int margin = 0;
#else
- int margin = FRAME_TOP_MARGIN_HEIGHT (f);
+ const int margin = FRAME_TOP_MARGIN_HEIGHT (f);
#endif
- int face_id =
+ const int face_id =
!NILP (Vface_remapping_alist)
? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
: INTERNAL_BORDER_FACE_ID;
@@ -1455,7 +1455,7 @@ x_draw_fringe_bitmap (struct window *w, struct glyph_row *row, struct draw_fring
Drawable drawable = FRAME_X_DRAWABLE (f);
char *bits;
Pixmap pixmap, clipmask = (Pixmap) 0;
- int depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
+ const int depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
XGCValues gcv;
if (p->wd > 8)
@@ -1704,7 +1704,7 @@ static void
x_set_glyph_string_clipping (struct glyph_string *s)
{
XRectangle *r = s->clip;
- int n = get_glyph_string_clip_rects (s, r, 2);
+ const int n = get_glyph_string_clip_rects (s, r, 2);
if (n > 0)
x_set_clip_rectangles (s->f, s->gc, r, n);
@@ -1797,7 +1797,7 @@ x_draw_glyph_string_background (struct glyph_string *s, bool force_p)
shouldn't be drawn in the first place. */
if (!s->background_filled_p)
{
- int box_line_width = max (s->face->box_line_width, 0);
+ const int box_line_width = max (s->face->box_line_width, 0);
if (s->stippled_p)
{
@@ -1908,15 +1908,15 @@ x_draw_composite_glyph_string_foreground (struct glyph_string *s)
}
else if (! s->first_glyph->u.cmp.automatic)
{
- int y = s->ybase;
+ const int y = s->ybase;
for (i = 0, j = s->cmp_from; i < s->nchars; i++, j++)
/* TAB in a composition means display glyphs with padding
space on the left or right. */
if (COMPOSITION_GLYPH (s->cmp, j) != '\t')
{
- int xx = x + s->cmp->offsets[j * 2];
- int yy = y - s->cmp->offsets[j * 2 + 1];
+ const int xx = x + s->cmp->offsets[j * 2];
+ const int yy = y - s->cmp->offsets[j * 2 + 1];
font->driver->draw (s, j, j + 1, xx, yy, false);
if (s->face->overstrike)
@@ -5516,7 +5516,7 @@ x_send_scroll_bar_event (Lisp_Object window, enum scroll_bar_part part,
struct frame *f = XFRAME (w->frame);
intptr_t iw = (intptr_t) w;
verify (INTPTR_WIDTH <= 64);
- int sign_shift = INTPTR_WIDTH - 32;
+ const int sign_shift = INTPTR_WIDTH - 32;
block_input ();
--
2.21.0
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 31 Mar 2019 22:38:05 GMT)
Full text and
rfc822 format available.
Message #17 received at 35062 <at> debbugs.gnu.org (full text, mbox):
* src/gtkutil.c: remove unnecessary comparison
Fixes LGTM warnings:
* Comparison is always false because min_cols <= 0.
* Comparison is always false because min_rows <= 0.
---
src/gtkutil.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/gtkutil.c b/src/gtkutil.c
index 4bd73b1a6d1..64bc248d650 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -1401,7 +1401,7 @@ x_wm_set_size_hint (struct frame *f, long int flags, bool user_position)
GdkGeometry size_hints;
gint hint_flags = 0;
int base_width, base_height;
- int min_rows = 0, min_cols = 0;
+ const int min_rows = 0, min_cols = 0;
int win_gravity = f->win_gravity;
Lisp_Object fs_state, frame;
int scale = xg_get_scale (f);
@@ -1450,9 +1450,6 @@ x_wm_set_size_hint (struct frame *f, long int flags, bool user_position)
base_height = FRAME_TEXT_LINES_TO_PIXEL_HEIGHT (f, 1)
+ FRAME_MENUBAR_HEIGHT (f) + FRAME_TOOLBAR_HEIGHT (f);
- if (min_cols > 0) --min_cols; /* We used one col in base_width = ... 1); */
- if (min_rows > 0) --min_rows; /* We used one row in base_height = ... 1); */
-
size_hints.base_width = base_width;
size_hints.base_height = base_height;
size_hints.min_width = base_width + min_cols * FRAME_COLUMN_WIDTH (f);
--
2.21.0
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 31 Mar 2019 22:38:05 GMT)
Full text and
rfc822 format available.
Message #20 received at 35062 <at> debbugs.gnu.org (full text, mbox):
* pdumper.c: don't compare unsigned to less-than-zero
Fixes 2 LGTM warnings: "Pointless comparison of unsigned value to zero."
---
src/pdumper.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/pdumper.c b/src/pdumper.c
index a9b3732a2d4..5407154fb2d 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -4434,7 +4434,6 @@ dump_anonymous_allocate (void *base,
static void
dump_anonymous_release (void *addr, size_t size)
{
- eassert (size >= 0);
#if VM_SUPPORTED == VM_MS_WINDOWS
(void) size;
if (!VirtualFree (addr, 0, MEM_RELEASE))
@@ -4584,7 +4583,6 @@ dump_map_file (void *base, int fd, off_t offset, size_t size,
static void
dump_unmap_file (void *addr, size_t size)
{
- eassert (size >= 0);
#if !VM_SUPPORTED
(void) addr;
(void) size;
--
2.21.0
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 01 Apr 2019 04:38:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
> Date: Mon, 1 Apr 2019 01:37:38 +0300
>
> These are mostly fixes of some of LGTM warnings
> https://lgtm.com/projects/g/emacs-mirror/emacs/alerts/?mode=tree
>
> Except the second patch, where I initially wanted to fix one warning,
> and as part of it I had to constify a variable to see that it is indeed
> immutable. And then I figured I could search through the code and find
> more similar places, where variables weren't marked as const. I like
> this cleanup because it is simple and trivially testable (i.e. if it
> compiles, then it's fine). FTR: there's still lots of opportunities for
> constification, I just stopped at some point.
Thanks.
I think the general policy is not to fix those except when making
other changes in the same function, but I will let others comment.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 01 Apr 2019 13:29:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 35062 <at> debbugs.gnu.org (full text, mbox):
>>>>> On Mon, 01 Apr 2019 07:37:23 +0300, Eli Zaretskii <eliz <at> gnu.org> said:
>> From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru> Date: Mon, 1
>> Apr 2019 01:37:38 +0300
>>
>> These are mostly fixes of some of LGTM warnings
>> https://lgtm.com/projects/g/emacs-mirror/emacs/alerts/?mode=tree
>>
>> Except the second patch, where I initially wanted to fix one
>> warning, and as part of it I had to constify a variable to see
>> that it is indeed immutable. And then I figured I could search
>> through the code and find more similar places, where variables
>> weren't marked as const. I like this cleanup because it is
>> simple and trivially testable (i.e. if it compiles, then it's
>> fine). FTR: there's still lots of opportunities for
>> constification, I just stopped at some point.
Eli> Thanks.
Eli> I think the general policy is not to fix those except when
Eli> making other changes in the same function, but I will let
Eli> others comment.
Iʼd prefer it if the effort went to determining if eg the alert for
'type = 2' below was correct or not, proving the constness of
variables is what we have a compiler for.
xterm.c:5346
if (XSCROLL_BAR (bar)->x_window == window_id
&& FRAME_X_DISPLAY (XFRAME (frame)) == display
&& (type = 2
|| (type == 1 && XSCROLL_BAR (bar)->horizontal)
|| (type == 0 && !XSCROLL_BAR (bar)->horizontal)))
return XSCROLL_BAR (bar);
Robert
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 01 Apr 2019 13:37:01 GMT)
Full text and
rfc822 format available.
Message #29 received at 35062 <at> debbugs.gnu.org (full text, mbox):
On Пн, Apr 1, 2019 at 15:27, Robert Pluim <rpluim <at> gmail.com> wrote:
>>>>>> On Mon, 01 Apr 2019 07:37:23 +0300, Eli Zaretskii
>>>>>> <eliz <at> gnu.org> said:
>
> >> From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru> Date: Mon, 1
> >> Apr 2019 01:37:38 +0300
> >>
> >> These are mostly fixes of some of LGTM warnings
> >>
> https://lgtm.com/projects/g/emacs-mirror/emacs/alerts/?mode=tree
> >>
> >> Except the second patch, where I initially wanted to fix one
> >> warning, and as part of it I had to constify a variable to see
> >> that it is indeed immutable. And then I figured I could search
> >> through the code and find more similar places, where variables
> >> weren't marked as const. I like this cleanup because it is
> >> simple and trivially testable (i.e. if it compiles, then it's
> >> fine). FTR: there's still lots of opportunities for
> >> constification, I just stopped at some point.
>
> Eli> Thanks.
>
> Eli> I think the general policy is not to fix those except when
> Eli> making other changes in the same function, but I will let
> Eli> others comment.
>
> Iʼd prefer it if the effort went to determining if eg the alert for
> 'type = 2' below was correct or not, proving the constness of
> variables is what we have a compiler for.
>
> xterm.c:5346
>
> if (XSCROLL_BAR (bar)->x_window == window_id
> && FRAME_X_DISPLAY (XFRAME (frame)) == display
> && (type = 2
> || (type == 1 && XSCROLL_BAR (bar)->horizontal)
> || (type == 0 && !XSCROLL_BAR (bar)->horizontal)))
> return XSCROLL_BAR (bar);
>
> Robert
Well, not everything at once! :) I afraid that if I fix lots of
warnings in one patch-set, it may get stuck in review because of the
amount of changes; besides it's easier for my sanity to send small
patchsets because mailing-list based projects in general tend not to
accept patches too quickly.
Also note: the constness here is not for compiler but for developers.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 01 Apr 2019 13:42:04 GMT)
Full text and
rfc822 format available.
Message #32 received at 35062 <at> debbugs.gnu.org (full text, mbox):
Btw, I'm okay if somebody else too gonna take down more warnings from
the lgtm site :)
On Пн, Apr 1, 2019 at 16:35, Konstantin Kharlamov
<hi-angel <at> yandex.ru> wrote:
>
>
> On Пн, Apr 1, 2019 at 15:27, Robert Pluim <rpluim <at> gmail.com> wrote:
>>>>>>> On Mon, 01 Apr 2019 07:37:23 +0300, Eli Zaretskii
>>>>>>> <eliz <at> gnu.org> said:
>>
>> >> From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru> Date: Mon, 1
>> >> Apr 2019 01:37:38 +0300
>> >>
>> >> These are mostly fixes of some of LGTM warnings
>> >>
>> https://lgtm.com/projects/g/emacs-mirror/emacs/alerts/?mode=tree
>> >>
>> >> Except the second patch, where I initially wanted to fix one
>> >> warning, and as part of it I had to constify a variable to see
>> >> that it is indeed immutable. And then I figured I could search
>> >> through the code and find more similar places, where variables
>> >> weren't marked as const. I like this cleanup because it is
>> >> simple and trivially testable (i.e. if it compiles, then it's
>> >> fine). FTR: there's still lots of opportunities for
>> >> constification, I just stopped at some point.
>>
>> Eli> Thanks.
>>
>> Eli> I think the general policy is not to fix those except when
>> Eli> making other changes in the same function, but I will let
>> Eli> others comment.
>>
>> Iʼd prefer it if the effort went to determining if eg the alert for
>> 'type = 2' below was correct or not, proving the constness of
>> variables is what we have a compiler for.
>>
>> xterm.c:5346
>>
>> if (XSCROLL_BAR (bar)->x_window == window_id
>> && FRAME_X_DISPLAY (XFRAME (frame)) == display
>> && (type = 2
>> || (type == 1 && XSCROLL_BAR (bar)->horizontal)
>> || (type == 0 && !XSCROLL_BAR (bar)->horizontal)))
>> return XSCROLL_BAR (bar);
>>
>> Robert
>
> Well, not everything at once! :) I afraid that if I fix lots of
> warnings in one patch-set, it may get stuck in review because of the
> amount of changes; besides it's easier for my sanity to send small
> patchsets because mailing-list based projects in general tend not to
> accept patches too quickly.
>
> Also note: the constness here is not for compiler but for developers.
>
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 01 Apr 2019 13:44:01 GMT)
Full text and
rfc822 format available.
Message #35 received at 35062 <at> debbugs.gnu.org (full text, mbox):
>>>>> On Mon, 01 Apr 2019 16:35:50 +0300, Konstantin Kharlamov <hi-angel <at> yandex.ru> said:
>> xterm.c:5346
>>
>> if (XSCROLL_BAR (bar)->x_window == window_id && FRAME_X_DISPLAY
>> (XFRAME (frame)) == display && (type = 2 || (type == 1 &&
>> XSCROLL_BAR (bar)->horizontal) || (type == 0 && !XSCROLL_BAR
>> (bar)->horizontal))) return XSCROLL_BAR (bar);
>>
>> Robert
Konstantin> Well, not everything at once! :) I afraid that if I
Konstantin> fix lots of warnings in one patch-set, it may get
Konstantin> stuck in review because of the amount of changes;
Konstantin> besides it's easier for my sanity to send small
Konstantin> patchsets because mailing-list based projects in
Konstantin> general tend not to accept patches too quickly.
Thatʼs why you concentrate on things that look like errors :-)
Konstantin> Also note: the constness here is not for compiler but
Konstantin> for developers.
I donʼt see how developers are particularly helped in this particular
case, but then again they're not harmed either.
Robert
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 01 Apr 2019 13:52:01 GMT)
Full text and
rfc822 format available.
Message #38 received at 35062 <at> debbugs.gnu.org (full text, mbox):
On Пн, Apr 1, 2019 at 15:43, Robert Pluim <rpluim <at> gmail.com> wrote:
>>>>>> On Mon, 01 Apr 2019 16:35:50 +0300, Konstantin Kharlamov
>>>>>> <hi-angel <at> yandex.ru> said:
> >> xterm.c:5346
> >>
> >> if (XSCROLL_BAR (bar)->x_window == window_id && FRAME_X_DISPLAY
> >> (XFRAME (frame)) == display && (type = 2 || (type == 1 &&
> >> XSCROLL_BAR (bar)->horizontal) || (type == 0 && !XSCROLL_BAR
> >> (bar)->horizontal))) return XSCROLL_BAR (bar);
> >>
> >> Robert
>
> Konstantin> Well, not everything at once! :) I afraid that if I
> Konstantin> fix lots of warnings in one patch-set, it may get
> Konstantin> stuck in review because of the amount of changes;
> Konstantin> besides it's easier for my sanity to send small
> Konstantin> patchsets because mailing-list based projects in
> Konstantin> general tend not to accept patches too quickly.
>
> Thatʼs why you concentrate on things that look like errors :-)
>
> Konstantin> Also note: the constness here is not for compiler but
> Konstantin> for developers.
>
> I donʼt see how developers are particularly helped in this particular
> case, but then again they're not harmed either.
For illustration you can take a look at patch 3/4 here. There, I
constify min_rows and min_cols; and afterwards I remove a paragraph
that compares them to a number that wasn't assigned there. This allows
to not look through the code before the comparison because it's
immediately obvious: these variables are never changed.
This is true for reading the code as well, especially when you're
debugging a problem: you may often wonder "okay, when was the last time
that variable changed, could it be invalid here?". Then, if it's
"const" you immediately know the answer, whereas otherwise you have to
search through all usages of that variable to see when was the last
time it changed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 01 Apr 2019 14:35:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> From: Robert Pluim <rpluim <at> gmail.com>
> Cc: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>, 35062 <at> debbugs.gnu.org
> Date: Mon, 01 Apr 2019 15:27:59 +0200
>
> Iʼd prefer it if the effort went to determining if eg the alert for
> 'type = 2' below was correct or not
Isn't it clear?
Btw, AFAICT, that function is _always_ called with type == 2 in
xterm.c, never with any other value. Only its other implementations
can be called with a different value of TYPE.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 01 Apr 2019 15:06:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 35062 <at> debbugs.gnu.org (full text, mbox):
>>>>> On Mon, 01 Apr 2019 17:34:46 +0300, Eli Zaretskii <eliz <at> gnu.org> said:
>> From: Robert Pluim <rpluim <at> gmail.com> Cc: Konstantin Kharlamov
>> <Hi-Angel <at> yandex.ru>, 35062 <at> debbugs.gnu.org Date: Mon, 01 Apr
>> 2019 15:27:59 +0200
>>
>> Iʼd prefer it if the effort went to determining if eg the alert
>> for 'type = 2' below was correct or not
Eli> Isn't it clear?
No, because you had to do your analysis below first to see if using '=='
changed the meaning of the code.
Eli> Btw, AFAICT, that function is _always_ called with type == 2
Eli> in xterm.c, never with any other value. Only its other
Eli> implementations can be called with a different value of TYPE.
BTW, the w32term.c implementation has the same typo :-)
Perhaps we can just delete most of that condition (I canʼt test at the
moment, my X11 machine is down).
diff --git a/src/xterm.c b/src/xterm.c
index f90d6713b0..a52f2c0862 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -5342,10 +5342,7 @@ x_window_to_scroll_bar (Display *display, Window window_id, int type)
! NILP (bar));
bar = XSCROLL_BAR (bar)->next)
if (XSCROLL_BAR (bar)->x_window == window_id
- && FRAME_X_DISPLAY (XFRAME (frame)) == display
- && (type = 2
- || (type == 1 && XSCROLL_BAR (bar)->horizontal)
- || (type == 0 && !XSCROLL_BAR (bar)->horizontal)))
+ && FRAME_X_DISPLAY (XFRAME (frame)) == display)
return XSCROLL_BAR (bar);
}
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 01 Apr 2019 17:38:01 GMT)
Full text and
rfc822 format available.
Message #47 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> From: Robert Pluim <rpluim <at> gmail.com>
> Cc: 35062 <at> debbugs.gnu.org, Hi-Angel <at> yandex.ru
> Date: Mon, 01 Apr 2019 17:04:52 +0200
>
> Eli> Btw, AFAICT, that function is _always_ called with type == 2
> Eli> in xterm.c, never with any other value. Only its other
> Eli> implementations can be called with a different value of TYPE.
>
> BTW, the w32term.c implementation has the same typo :-)
Yes, I fixed them both right after I wrote that.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 01 Apr 2019 22:38:01 GMT)
Full text and
rfc822 format available.
Message #50 received at 35062 <at> debbugs.gnu.org (full text, mbox):
Konstantin Kharlamov <Hi-Angel <at> yandex.ru> writes:
> * src/gtkutil.c: remove unnecessary comparison
> @@ -1401,7 +1401,7 @@ x_wm_set_size_hint (struct frame *f, long int flags, bool user_position)
> - int min_rows = 0, min_cols = 0;
> + const int min_rows = 0, min_cols = 0;
> - if (min_cols > 0) --min_cols; /* We used one col in base_width = ... 1); */
> - if (min_rows > 0) --min_rows; /* We used one row in base_height = ... 1); */
> -
> size_hints.base_width = base_width;
> size_hints.base_height = base_height;
> size_hints.min_width = base_width + min_cols * FRAME_COLUMN_WIDTH (f);
If min_cols is always 0, then what use is the
"+ min_cols * FRAME_COLUMN_WIDTH (f)"?
(and a similar question arises for min_cols and min_rows in
update_wm_hints of widget.c)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Tue, 02 Apr 2019 00:10:01 GMT)
Full text and
rfc822 format available.
Message #53 received at 35062 <at> debbugs.gnu.org (full text, mbox):
В Пн, апр 1, 2019 at 18:37, Noam Postavsky <npostavs <at> gmail.com>
написал:
> Konstantin Kharlamov <Hi-Angel <at> yandex.ru> writes:
>
>> * src/gtkutil.c: remove unnecessary comparison
>
>> @@ -1401,7 +1401,7 @@ x_wm_set_size_hint (struct frame *f, long int
>> flags, bool user_position)
>
>> - int min_rows = 0, min_cols = 0;
>> + const int min_rows = 0, min_cols = 0;
>
>> - if (min_cols > 0) --min_cols; /* We used one col in base_width =
>> ... 1); */
>> - if (min_rows > 0) --min_rows; /* We used one row in base_height
>> = ... 1); */
>> -
>> size_hints.base_width = base_width;
>> size_hints.base_height = base_height;
>> size_hints.min_width = base_width + min_cols *
>> FRAME_COLUMN_WIDTH (f);
>
> If min_cols is always 0, then what use is the
> "+ min_cols * FRAME_COLUMN_WIDTH (f)"?
>
> (and a similar question arises for min_cols and min_rows in
> update_wm_hints of widget.c)
Good point. I did some digging in git-log/git-blame: once upon a time
there was a check_frame_size (f, &min_cols, &min_rows, 0) call. Then in
commit 3477e27021dbe9366c3c1aaba80feb72f1138b29 for some reason this
call was removed. Reason is unclear because the description only says
multiple times "remove check_frame_size", and the commit itself is so
*GIGANTIC* that when I held PgDown, it took me 9 seconds just to scroll
it to the bottom on a 1366×768 display. But the commit is dated by
2014, so given it's still there, let's hope it's correct.
Then I guess, it makes sense to remove the multiplication by zero,
because it's a noop anyway. I gonna make changes in gtkutil.c in some
minutes, but what do I do with widget.c? Do I just send the patch here?
I'm asking because Emacs has unusual workflow regarding patches
sending. E.g. in other mailing-list based projects I'd just resend the
series. In gitlab/github it's much simpler: I just push the new commit.
But in Emacs one has to create a debbugs report, then send patches
there; and I already have this "report" so I'm confused.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Tue, 02 Apr 2019 00:24:02 GMT)
Full text and
rfc822 format available.
Message #56 received at 35062 <at> debbugs.gnu.org (full text, mbox):
* src/gtkutil.c:
remove "always false" comparison
remove multiplication by zero, it's a noop anyway
remove min_cols and min_rows as it's now unused
Fixes LGTM warnings:
* Comparison is always false because min_cols <= 0.
* Comparison is always false because min_rows <= 0.
---
v2: remove the min_rows/min_cols whatsoever
src/gtkutil.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/gtkutil.c b/src/gtkutil.c
index 4bd73b1a6d1..b130692c87a 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -1401,7 +1401,6 @@ x_wm_set_size_hint (struct frame *f, long int flags, bool user_position)
GdkGeometry size_hints;
gint hint_flags = 0;
int base_width, base_height;
- int min_rows = 0, min_cols = 0;
int win_gravity = f->win_gravity;
Lisp_Object fs_state, frame;
int scale = xg_get_scale (f);
@@ -1450,13 +1449,10 @@ x_wm_set_size_hint (struct frame *f, long int flags, bool user_position)
base_height = FRAME_TEXT_LINES_TO_PIXEL_HEIGHT (f, 1)
+ FRAME_MENUBAR_HEIGHT (f) + FRAME_TOOLBAR_HEIGHT (f);
- if (min_cols > 0) --min_cols; /* We used one col in base_width = ... 1); */
- if (min_rows > 0) --min_rows; /* We used one row in base_height = ... 1); */
-
size_hints.base_width = base_width;
size_hints.base_height = base_height;
- size_hints.min_width = base_width + min_cols * FRAME_COLUMN_WIDTH (f);
- size_hints.min_height = base_height + min_rows * FRAME_LINE_HEIGHT (f);
+ size_hints.min_width = base_width;
+ size_hints.min_height = base_height;
/* These currently have a one to one mapping with the X values, but I
don't think we should rely on that. */
--
2.21.0
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Tue, 02 Apr 2019 14:47:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Tue, 02 Apr 2019 03:09:14 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: 35062 <at> debbugs.gnu.org
>
> what do I do with widget.c? Do I just send the patch here?
> I'm asking because Emacs has unusual workflow regarding patches
> sending. E.g. in other mailing-list based projects I'd just resend the
> series. In gitlab/github it's much simpler: I just push the new commit.
> But in Emacs one has to create a debbugs report, then send patches
> there; and I already have this "report" so I'm confused.
The rule is very simple: if the other change is for the same or a
similar issue, just send another patch to the same issue ticket;
otherwise file a separate issue. (But no one will be necessarily mad
at you if you send to the same ticket anyway, it just runs a higher
risk of falling through cracks. IOW, don't worry too much about this
stuff, just use common sense; we are not as rigid in our procedures as
some other projects.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Tue, 02 Apr 2019 20:51:01 GMT)
Full text and
rfc822 format available.
Message #62 received at 35062 <at> debbugs.gnu.org (full text, mbox):
* widget.c: multiplication by zero always equals zero
---
src/widget.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/widget.c b/src/widget.c
index c695bd5f305..508974dd46f 100644
--- a/src/widget.c
+++ b/src/widget.c
@@ -297,7 +297,6 @@ update_wm_hints (EmacsFrame ew)
int char_height;
int base_width;
int base_height;
- int min_rows = 0, min_cols = 0;
/* This happens when the frame is just created. */
if (! wmshell) return;
@@ -323,8 +322,8 @@ update_wm_hints (EmacsFrame ew)
XtNbaseHeight, (XtArgVal) base_height,
XtNwidthInc, (XtArgVal) (frame_resize_pixelwise ? 1 : cw),
XtNheightInc, (XtArgVal) (frame_resize_pixelwise ? 1 : ch),
- XtNminWidth, (XtArgVal) (base_width + min_cols * cw),
- XtNminHeight, (XtArgVal) (base_height + min_rows * ch),
+ XtNminWidth, (XtArgVal) base_width,
+ XtNminHeight, (XtArgVal) base_height,
NULL);
}
--
2.21.0
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Tue, 02 Apr 2019 20:55:01 GMT)
Full text and
rfc822 format available.
Message #65 received at 35062 <at> debbugs.gnu.org (full text, mbox):
Thanks, done!
In other news: today I got a confirmation from FSF that assignment
process is complete. I guess that's it, I can officially contribute to
Emacs now…?
В Вт, апр 2, 2019 at 17:46, Eli Zaretskii <eliz <at> gnu.org>
написал:
>> Date: Tue, 02 Apr 2019 03:09:14 +0300
>> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>> Cc: 35062 <at> debbugs.gnu.org
>>
>> what do I do with widget.c? Do I just send the patch here?
>> I'm asking because Emacs has unusual workflow regarding patches
>> sending. E.g. in other mailing-list based projects I'd just resend
>> the
>> series. In gitlab/github it's much simpler: I just push the new
>> commit.
>> But in Emacs one has to create a debbugs report, then send patches
>> there; and I already have this "report" so I'm confused.
>
> The rule is very simple: if the other change is for the same or a
> similar issue, just send another patch to the same issue ticket;
> otherwise file a separate issue. (But no one will be necessarily mad
> at you if you send to the same ticket anyway, it just runs a higher
> risk of falling through cracks. IOW, don't worry too much about this
> stuff, just use common sense; we are not as rigid in our procedures as
> some other projects.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Wed, 03 Apr 2019 04:46:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Tue, 02 Apr 2019 23:54:42 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: npostavs <at> gmail.com, 35062 <at> debbugs.gnu.org
>
> In other news: today I got a confirmation from FSF that assignment
> process is complete. I guess that's it, I can officially contribute to
> Emacs now…?
Yes, welcome on board.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Fri, 05 Apr 2019 07:06:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
> Date: Tue, 2 Apr 2019 03:23:27 +0300
>
> * src/gtkutil.c:
> remove "always false" comparison
> remove multiplication by zero, it's a noop anyway
> remove min_cols and min_rows as it's now unused
>
> Fixes LGTM warnings:
> * Comparison is always false because min_cols <= 0.
> * Comparison is always false because min_rows <= 0.
> ---
> v2: remove the min_rows/min_cols whatsoever
Thanks, pushed. Please in the future write the commit log messages in
the ChangeLog-like style, see CONTRIBUTE for the details. I did that
this time, you can look at what I pushed to see an example of that
style in action.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Fri, 05 Apr 2019 07:18:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
> Date: Tue, 2 Apr 2019 23:49:58 +0300
>
> * widget.c: multiplication by zero always equals zero
> ---
> src/widget.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
Thanks, pushed to the master branch (with appropriate commit log
message).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Fri, 05 Apr 2019 22:19:01 GMT)
Full text and
rfc822 format available.
Message #77 received at 35062 <at> debbugs.gnu.org (full text, mbox):
On Пт, Apr 5, 2019 at 10:05, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
>> Date: Tue, 2 Apr 2019 03:23:27 +0300
>>
>> * src/gtkutil.c:
>> remove "always false" comparison
>> remove multiplication by zero, it's a noop anyway
>> remove min_cols and min_rows as it's now unused
>>
>> Fixes LGTM warnings:
>> * Comparison is always false because min_cols <= 0.
>> * Comparison is always false because min_rows <= 0.
>> ---
>> v2: remove the min_rows/min_cols whatsoever
>
> Thanks, pushed. Please in the future write the commit log messages in
> the ChangeLog-like style, see CONTRIBUTE for the details. I did that
> this time, you can look at what I pushed to see an example of that
> style in action.
Thanks, so, I'm looking right now to modify the rest of series and
resend it here, and… I think I miss something: the only difference of
my commit from CONTRIBUTE file is that I forgot to list the functions
changed.
I think I misunderstand something, because you completely rewrote the
commit messages compared to mine. Titles now are more vague, and for
some reason you removed a note that one of patches fixes warnings in
the code. And then you also added "Bug#35062", even though there's no
bug (the report existence is incidental, it's because bugtracker is
used to track patches in Emacs. But writing "Bug" will only confuse
readers — there's no bug).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 06 Apr 2019 07:10:01 GMT)
Full text and
rfc822 format available.
Message #80 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 06 Apr 2019 01:18:38 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: 35062 <at> debbugs.gnu.org
>
> >> * src/gtkutil.c:
> >> remove "always false" comparison
> >> remove multiplication by zero, it's a noop anyway
> >> remove min_cols and min_rows as it's now unused
> >>
> >> Fixes LGTM warnings:
> >> * Comparison is always false because min_cols <= 0.
> >> * Comparison is always false because min_rows <= 0.
> >> ---
> >> v2: remove the min_rows/min_cols whatsoever
> >
> > Thanks, pushed. Please in the future write the commit log messages in
> > the ChangeLog-like style, see CONTRIBUTE for the details. I did that
> > this time, you can look at what I pushed to see an example of that
> > style in action.
>
> Thanks, so, I'm looking right now to modify the rest of series and
> resend it here, and… I think I miss something: the only difference of
> my commit from CONTRIBUTE file is that I forgot to list the functions
> changed.
That's right. And also the lack of the bug number, see below.
> I think I misunderstand something, because you completely rewrote the
> commit messages compared to mine. Titles now are more vague, and for
> some reason you removed a note that one of patches fixes warnings in
> the code.
That's just my personal preferences of style, as there's always a
judgment call regarding the description of the changes. We don't have
to agree about that, and if your log message was in the right format,
I probably wouldn't have bothered changing their text.
I can explain my preference: your text described in a very detailed
form something that was acutely visible from the diffs themselves,
whereas my text only describes the motivation, leaving the rest to the
diffs which speak for themselves.
The part about LGTM warnings didn't seem important enough to me: LGTM
is not a compiler, and without a reference to what it is, the text is
a small riddle in itself. More importantly, I don't think we need any
justification for removing variables initialized to zero and never
changed.
Again, these are my personal preferences, not something we require
from each contributor. So if you disagree, you can keep your style,
and only adjust the form.
> And then you also added "Bug#35062", even though there's no
> bug (the report existence is incidental, it's because bugtracker is
> used to track patches in Emacs.
References to bug numbers should always be in the log message, because
that allows an easy way of reading relevant discussions recorded by
the bug tracker. This _is_ in CONTRIBUTE, btw.
> But writing "Bug" will only confuse readers — there's no bug).
"Bug" here doesn't mean a literal bug, it means an issue recorded by
the bug tracker.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 06 Apr 2019 07:26:01 GMT)
Full text and
rfc822 format available.
Message #83 received at 35062 <at> debbugs.gnu.org (full text, mbox):
Konstantin Kharlamov <hi-angel <at> yandex.ru> writes:
Hi Konstantin,
> And then you also added "Bug#35062", even though there's no bug (the
> report existence is incidental, it's because bugtracker is used to
> track patches in Emacs. But writing "Bug" will only confuse readers —
> there's no bug).
"Bug#nnnnn" is a pattern to identify references to debbugs.gnu.org in
files or git logs. Used by functions like `bug-reference-mode' or
`debbugs-browse-mode'.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 07 Apr 2019 02:04:01 GMT)
Full text and
rfc822 format available.
Message #86 received at 35062 <at> debbugs.gnu.org (full text, mbox):
В Сб, апр 6, 2019 at 10:09, Eli Zaretskii <eliz <at> gnu.org>
написал:
>> Date: Sat, 06 Apr 2019 01:18:38 +0300
>> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>> Cc: 35062 <at> debbugs.gnu.org
>>
>> >> * src/gtkutil.c:
>> >> remove "always false" comparison
>> >> remove multiplication by zero, it's a noop anyway
>> >> remove min_cols and min_rows as it's now unused
>> >>
>> >> Fixes LGTM warnings:
>> >> * Comparison is always false because min_cols <= 0.
>> >> * Comparison is always false because min_rows <= 0.
>> >> ---
>> >> v2: remove the min_rows/min_cols whatsoever
>> >
>> > Thanks, pushed. Please in the future write the commit log
>> messages in
>> > the ChangeLog-like style, see CONTRIBUTE for the details. I did
>> that
>> > this time, you can look at what I pushed to see an example of that
>> > style in action.
>>
>> Thanks, so, I'm looking right now to modify the rest of series and
>> resend it here, and… I think I miss something: the only
>> difference of
>> my commit from CONTRIBUTE file is that I forgot to list the
>> functions
>> changed.
>
> That's right. And also the lack of the bug number, see below.
>
>> I think I misunderstand something, because you completely rewrote
>> the
>> commit messages compared to mine. Titles now are more vague, and for
>> some reason you removed a note that one of patches fixes warnings in
>> the code.
>
> That's just my personal preferences of style, as there's always a
> judgment call regarding the description of the changes. We don't have
> to agree about that, and if your log message was in the right format,
> I probably wouldn't have bothered changing their text.
>
> I can explain my preference: your text described in a very detailed
> form something that was acutely visible from the diffs themselves,
> whereas my text only describes the motivation, leaving the rest to the
> diffs which speak for themselves.
Thanks!
> The part about LGTM warnings didn't seem important enough to me: LGTM
> is not a compiler, and without a reference to what it is, the text is
> a small riddle in itself. More importantly, I don't think we need any
> justification for removing variables initialized to zero and never
> changed.
Well, one can find what LGTM through a search engine, or for that
matter by using "BUG#XXX" to find this discussion. But whatever.
> Again, these are my personal preferences, not something we require
> from each contributor. So if you disagree, you can keep your style,
> and only adjust the form.
>
>> And then you also added "Bug#35062", even though there's no
>> bug (the report existence is incidental, it's because bugtracker is
>> used to track patches in Emacs.
>
> References to bug numbers should always be in the log message, because
> that allows an easy way of reading relevant discussions recorded by
> the bug tracker. This _is_ in CONTRIBUTE, btw.
Oh, thanks, I missed that.
>> But writing "Bug" will only confuse readers — there's no bug).
>
> "Bug" here doesn't mean a literal bug, it means an issue recorded by
> the bug tracker.
I see, still it doesn't mean arbitrary readers won't be confused.
git-logs are being read by lots of different people, most of them are
not even contributors, could be e.g. users wondering whether it's worth
to re-build their Emacs-from-git.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 07 Apr 2019 02:14:01 GMT)
Full text and
rfc822 format available.
Message #89 received at 35062 <at> debbugs.gnu.org (full text, mbox):
* src/xterm.c (x_set_frame_alpha): due to preceding checks it is bound
to be 0 ≤ alpha ≤ 1. (Bug#35062)
---
v3: mention functions changed in commit messages, mention the bug
number, and don't mention that it fixes a warning since intention of
changes is clear either way.
src/xterm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/xterm.c b/src/xterm.c
index f90d6713b02..e8f1e576a38 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -931,7 +931,7 @@ x_set_frame_alpha (struct frame *f)
return;
else if (alpha > 1.0)
alpha = 1.0;
- else if (0.0 <= alpha && alpha < alpha_min && alpha_min <= 1.0)
+ else if (alpha < alpha_min && alpha_min <= 1.0)
alpha = alpha_min;
opac = alpha * OPAQUE;
--
2.21.0
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 07 Apr 2019 02:14:02 GMT)
Full text and
rfc822 format available.
Message #92 received at 35062 <at> debbugs.gnu.org (full text, mbox):
* src/xterm.c (x_cr_draw_image, x_update_begin,
x_clear_under_internal_border, x_draw_fringe_bitmap,
x_set_glyph_string_clipping, x_draw_glyph_string_background,
x_draw_composite_glyph_string_foreground, x_send_scroll_bar_event): make
code easier to follow by making explicit that some variables are
immutable. (Bug#35062)
---
v3: mention functions changed in commit messages, mention the bug
number, and don't mention that it fixes a warning since intention of
changes is clear either way.
src/xterm.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/src/xterm.c b/src/xterm.c
index e8f1e576a38..8354ce00700 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -495,9 +495,9 @@ x_cr_draw_image (struct frame *f, GC gc, cairo_surface_t *image,
cairo_fill_preserve (cr);
}
- int orig_image_width = cairo_image_surface_get_width (image);
+ const int orig_image_width = cairo_image_surface_get_width (image);
if (image_width == 0) image_width = orig_image_width;
- int orig_image_height = cairo_image_surface_get_height (image);
+ const int orig_image_height = cairo_image_surface_get_height (image);
if (image_height == 0) image_height = orig_image_height;
cairo_pattern_t *pattern = cairo_pattern_create_for_surface (image);
@@ -1006,7 +1006,7 @@ x_update_begin (struct frame *f)
if (FRAME_GTK_WIDGET (f))
{
GdkWindow *w = gtk_widget_get_window (FRAME_GTK_WIDGET (f));
- int scale = xg_get_scale (f);
+ const int scale = xg_get_scale (f);
width = scale * gdk_window_get_width (w);
height = scale * gdk_window_get_height (w);
}
@@ -1300,15 +1300,15 @@ x_clear_under_internal_border (struct frame *f)
{
if (FRAME_INTERNAL_BORDER_WIDTH (f) > 0)
{
- int border = FRAME_INTERNAL_BORDER_WIDTH (f);
- int width = FRAME_PIXEL_WIDTH (f);
- int height = FRAME_PIXEL_HEIGHT (f);
+ const int border = FRAME_INTERNAL_BORDER_WIDTH (f);
+ const int width = FRAME_PIXEL_WIDTH (f);
+ const int height = FRAME_PIXEL_HEIGHT (f);
#ifdef USE_GTK
- int margin = 0;
+ const int margin = 0;
#else
- int margin = FRAME_TOP_MARGIN_HEIGHT (f);
+ const int margin = FRAME_TOP_MARGIN_HEIGHT (f);
#endif
- int face_id =
+ const int face_id =
!NILP (Vface_remapping_alist)
? lookup_basic_face (NULL, f, INTERNAL_BORDER_FACE_ID)
: INTERNAL_BORDER_FACE_ID;
@@ -1455,7 +1455,7 @@ x_draw_fringe_bitmap (struct window *w, struct glyph_row *row, struct draw_fring
Drawable drawable = FRAME_X_DRAWABLE (f);
char *bits;
Pixmap pixmap, clipmask = (Pixmap) 0;
- int depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
+ const int depth = DefaultDepthOfScreen (FRAME_X_SCREEN (f));
XGCValues gcv;
if (p->wd > 8)
@@ -1704,7 +1704,7 @@ static void
x_set_glyph_string_clipping (struct glyph_string *s)
{
XRectangle *r = s->clip;
- int n = get_glyph_string_clip_rects (s, r, 2);
+ const int n = get_glyph_string_clip_rects (s, r, 2);
if (n > 0)
x_set_clip_rectangles (s->f, s->gc, r, n);
@@ -1797,7 +1797,7 @@ x_draw_glyph_string_background (struct glyph_string *s, bool force_p)
shouldn't be drawn in the first place. */
if (!s->background_filled_p)
{
- int box_line_width = max (s->face->box_line_width, 0);
+ const int box_line_width = max (s->face->box_line_width, 0);
if (s->stippled_p)
{
@@ -1908,15 +1908,15 @@ x_draw_composite_glyph_string_foreground (struct glyph_string *s)
}
else if (! s->first_glyph->u.cmp.automatic)
{
- int y = s->ybase;
+ const int y = s->ybase;
for (i = 0, j = s->cmp_from; i < s->nchars; i++, j++)
/* TAB in a composition means display glyphs with padding
space on the left or right. */
if (COMPOSITION_GLYPH (s->cmp, j) != '\t')
{
- int xx = x + s->cmp->offsets[j * 2];
- int yy = y - s->cmp->offsets[j * 2 + 1];
+ const int xx = x + s->cmp->offsets[j * 2];
+ const int yy = y - s->cmp->offsets[j * 2 + 1];
font->driver->draw (s, j, j + 1, xx, yy, false);
if (s->face->overstrike)
@@ -5516,7 +5516,7 @@ x_send_scroll_bar_event (Lisp_Object window, enum scroll_bar_part part,
struct frame *f = XFRAME (w->frame);
intptr_t iw = (intptr_t) w;
verify (INTPTR_WIDTH <= 64);
- int sign_shift = INTPTR_WIDTH - 32;
+ const int sign_shift = INTPTR_WIDTH - 32;
block_input ();
--
2.21.0
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 07 Apr 2019 02:14:02 GMT)
Full text and
rfc822 format available.
Message #95 received at 35062 <at> debbugs.gnu.org (full text, mbox):
* pdumper.c (dump_anonymous_allocate, dump_map_file): don't compare
unsigned to less-than-zero (Bug#35062)
---
v3: mention functions changed in commit messages, mention the bug
number, and don't mention that it fixes a warning since intention of
changes is clear either way.
src/pdumper.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/pdumper.c b/src/pdumper.c
index a9b3732a2d4..5407154fb2d 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -4434,7 +4434,6 @@ dump_anonymous_allocate (void *base,
static void
dump_anonymous_release (void *addr, size_t size)
{
- eassert (size >= 0);
#if VM_SUPPORTED == VM_MS_WINDOWS
(void) size;
if (!VirtualFree (addr, 0, MEM_RELEASE))
@@ -4584,7 +4583,6 @@ dump_map_file (void *base, int fd, off_t offset, size_t size,
static void
dump_unmap_file (void *addr, size_t size)
{
- eassert (size >= 0);
#if !VM_SUPPORTED
(void) addr;
(void) size;
--
2.21.0
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 07 Apr 2019 02:46:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 07 Apr 2019 05:03:33 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: 35062 <at> debbugs.gnu.org
>
> >> But writing "Bug" will only confuse readers — there's no bug).
> >
> > "Bug" here doesn't mean a literal bug, it means an issue recorded by
> > the bug tracker.
>
> I see, still it doesn't mean arbitrary readers won't be confused.
> git-logs are being read by lots of different people, most of them are
> not even contributors, could be e.g. users wondering whether it's worth
> to re-build their Emacs-from-git.
These references are for the Emacs developers, the rest of the world
will have to adapt. It's not a big deal anyway.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 13 Apr 2019 08:07:02 GMT)
Full text and
rfc822 format available.
Message #101 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
> Date: Sun, 7 Apr 2019 05:13:29 +0300
>
> * src/xterm.c (x_set_frame_alpha): due to preceding checks it is bound
> to be 0 ≤ alpha ≤ 1. (Bug#35062)
> ---
> v3: mention functions changed in commit messages, mention the bug
> number, and don't mention that it fixes a warning since intention of
> changes is clear either way.
Thanks, I pushed a slightly different change along the same lines.
A minor nit: please avoid using non-ASCII characters in log entries,
if that can be avoided, to cater to terminals that don't support UTF-8
(the character you used is not in Latin-1).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 13 Apr 2019 08:12:01 GMT)
Full text and
rfc822 format available.
Message #104 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
> Date: Sun, 7 Apr 2019 05:13:31 +0300
>
> * pdumper.c (dump_anonymous_allocate, dump_map_file): don't compare
> unsigned to less-than-zero (Bug#35062)
A minor nit: the text after the function names should start with a
capital letter ("Don't compare..."), as any English sentence would.
> ---
> v3: mention functions changed in commit messages, mention the bug
> number, and don't mention that it fixes a warning since intention of
> changes is clear either way.
It is true that these assertions may look redundant, but pdumper.c
uses an unusually high level of safety features, so I'd like to have a
second opinion from its author.
Daniel, are you okay with removing these assertions?
> src/pdumper.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/src/pdumper.c b/src/pdumper.c
> index a9b3732a2d4..5407154fb2d 100644
> --- a/src/pdumper.c
> +++ b/src/pdumper.c
> @@ -4434,7 +4434,6 @@ dump_anonymous_allocate (void *base,
> static void
> dump_anonymous_release (void *addr, size_t size)
> {
> - eassert (size >= 0);
> #if VM_SUPPORTED == VM_MS_WINDOWS
> (void) size;
> if (!VirtualFree (addr, 0, MEM_RELEASE))
> @@ -4584,7 +4583,6 @@ dump_map_file (void *base, int fd, off_t offset, size_t size,
> static void
> dump_unmap_file (void *addr, size_t size)
> {
> - eassert (size >= 0);
> #if !VM_SUPPORTED
> (void) addr;
> (void) size;
> --
> 2.21.0
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 13 Apr 2019 08:16:02 GMT)
Full text and
rfc822 format available.
Message #107 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
> Date: Sun, 7 Apr 2019 05:13:30 +0300
>
> * src/xterm.c (x_cr_draw_image, x_update_begin,
> x_clear_under_internal_border, x_draw_fringe_bitmap,
> x_set_glyph_string_clipping, x_draw_glyph_string_background,
> x_draw_composite_glyph_string_foreground, x_send_scroll_bar_event): make
> code easier to follow by making explicit that some variables are
> immutable. (Bug#35062)
> ---
> v3: mention functions changed in commit messages, mention the bug
> number, and don't mention that it fixes a warning since intention of
> changes is clear either way.
I'm really struggling with these changes. My main problem is that I
don't see how using 'const' here improves the readability and clarity
of the code. IMO, if the variable's name doesn't state its purpose,
adding 'const' won't help much. And I think compilers nowadays are
smart enough to deduce this by themselves.
We had in the past similar issue with the 'register' qualifier;
nowadays we remove those whenever we change code that uses them.
Isn't it the same with 'const'?
Paul, could you please comment on these proposed changes?
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 13 Apr 2019 11:32:01 GMT)
Full text and
rfc822 format available.
Message #110 received at 35062 <at> debbugs.gnu.org (full text, mbox):
В Сб, апр 13, 2019 at 11:15, Eli Zaretskii <eliz <at> gnu.org>
написал:
>> From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
>> Date: Sun, 7 Apr 2019 05:13:30 +0300
>>
>> * src/xterm.c (x_cr_draw_image, x_update_begin,
>> x_clear_under_internal_border, x_draw_fringe_bitmap,
>> x_set_glyph_string_clipping, x_draw_glyph_string_background,
>> x_draw_composite_glyph_string_foreground, x_send_scroll_bar_event):
>> make
>> code easier to follow by making explicit that some variables are
>> immutable. (Bug#35062)
>> ---
>> v3: mention functions changed in commit messages, mention the bug
>> number, and don't mention that it fixes a warning since intention
>> of
>> changes is clear either way.
>
> I'm really struggling with these changes. My main problem is that I
> don't see how using 'const' here improves the readability and clarity
> of the code. IMO, if the variable's name doesn't state its purpose,
> adding 'const' won't help much. And I think compilers nowadays are
> smart enough to deduce this by themselves.
I've had an example somewhere at the beginning of this thread, so let
me quote myself
> For illustration you can take a look at patch 3/4 here. There, I
> constify min_rows and min_cols; and afterwards I remove a paragraph
> that compares them to a number that wasn't assigned there. This allows
> to not look through the code before the comparison because it's
> immediately obvious: these variables are never changed.
>
> This is true for reading the code as well, especially when you're
> debugging a problem: you may often wonder "okay, when was the last
> time
> that variable changed, could it be invalid here?". Then, if it's
> "const" you immediately know the answer, whereas otherwise you have to
> search through all usages of that variable to see when was the last
> time it changed.
Basically, less mutability means easier to read code. Usually, fully
immutable code (e.g. in languages such as Haskell) keeps algorithm as
clean as possible, whereas mutability adds points of mental burden
(i.e. because you don't know without going through whole function
whether variable was only assigned once, or was it changed somewhere).
Modern systems language Rust by default is immutable, and mutable
variables has to be explicitly marked as such (unlike it older
languages, where everything is mutable, and you have to `const`ify
things).
-----
Either way, if you folks really in doubt, I'm okay with dropping this
patch. It took me maybe 10 minutes to assemble it, so it's not like
there's much work went into it. I simply found a place for improvement
in code, and did that.
I'd like to take this as an opportunity to ask a question: I see Emacs
C code is using the old style where variables (mostly) are declared at
the beginning of a function. Is it just a legacy from C89 days (I hope
so), or is it a mandatory style?
I'm asking because if I gonna work on the code, I'd for sure like to
encapsulate variables as much as possible, which means I'd rather
declare them on the first use (as a bonus, this often may allow to
constify the variable too).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 13 Apr 2019 11:37:02 GMT)
Full text and
rfc822 format available.
Message #113 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 13 Apr 2019 14:30:46 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: Paul Eggert <eggert <at> cs.ucla.edu>, 35062 <at> debbugs.gnu.org
>
> I'd like to take this as an opportunity to ask a question: I see Emacs
> C code is using the old style where variables (mostly) are declared at
> the beginning of a function. Is it just a legacy from C89 days (I hope
> so), or is it a mandatory style?
It's legacy. Nowadays we allow declarations near the first usage, per
C99. But again, we only make changes in the order as part of other
code changes, not just by themselves.
> I'm asking because if I gonna work on the code, I'd for sure like to
> encapsulate variables as much as possible, which means I'd rather
> declare them on the first use (as a bonus, this often may allow to
> constify the variable too).
That's okay. We require C99 these days.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 13 Apr 2019 18:20:01 GMT)
Full text and
rfc822 format available.
Message #116 received at 35062 <at> debbugs.gnu.org (full text, mbox):
В Сб, апр 13, 2019 at 11:06, Eli Zaretskii <eliz <at> gnu.org>
написал:
>> From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
>> Date: Sun, 7 Apr 2019 05:13:29 +0300
>>
>> * src/xterm.c (x_set_frame_alpha): due to preceding checks it is
>> bound
>> to be 0 ≤ alpha ≤ 1. (Bug#35062)
>> ---
>> v3: mention functions changed in commit messages, mention the bug
>> number, and don't mention that it fixes a warning since intention
>> of
>> changes is clear either way.
>
> Thanks, I pushed a slightly different change along the same lines.
>
> A minor nit: please avoid using non-ASCII characters in log entries,
> if that can be avoided, to cater to terminals that don't support UTF-8
> (the character you used is not in Latin-1).
Err… I just looked at log — wait, what?? There is no my patch. You
crafted a patch of your own, incorporated my changes, and pushed as
your own. If I gonna pass someone this link
http://git.savannah.gnu.org/cgit/emacs.git/log/?qt=author&q=Konstantin
there is no my contribution.
More over: you didn't even tell me! Wow! This is not good.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 13 Apr 2019 18:26:02 GMT)
Full text and
rfc822 format available.
Message #119 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 13 Apr 2019 21:19:12 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: 35062 <at> debbugs.gnu.org
>
> Err… I just looked at log — wait, what?? There is no my patch. You
> crafted a patch of your own, incorporated my changes, and pushed as
> your own. If I gonna pass someone this link
> http://git.savannah.gnu.org/cgit/emacs.git/log/?qt=author&q=Konstantin
> there is no my contribution.
>
> More over: you didn't even tell me! Wow! This is not good.
What do you mean? Your name is in the log.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 13 Apr 2019 18:29:01 GMT)
Full text and
rfc822 format available.
Message #122 received at 35062 <at> debbugs.gnu.org (full text, mbox):
В Сб, апр 13, 2019 at 21:24, Eli Zaretskii <eliz <at> gnu.org>
написал:
>> Date: Sat, 13 Apr 2019 21:19:12 +0300
>> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>> Cc: 35062 <at> debbugs.gnu.org
>>
>> Err… I just looked at log — wait, what?? There is no my patch.
>> You
>> crafted a patch of your own, incorporated my changes, and pushed as
>> your own. If I gonna pass someone this link
>>
>> http://git.savannah.gnu.org/cgit/emacs.git/log/?qt=author&q=Konstantin
>> there is no my contribution.
>>
>> More over: you didn't even tell me! Wow! This is not good.
>
> What do you mean? Your name is in the log.
If you click the link above you won't find it. The only mention of my
name is in log message as "suggested-by". Which btw sounds like I
reported a bug "hey guys, I don't know how to code, but static analyzer
says this part can be improved, can anybody do it for me?".
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 13 Apr 2019 19:20:01 GMT)
Full text and
rfc822 format available.
Message #125 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 13 Apr 2019 21:28:39 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: 35062 <at> debbugs.gnu.org
>
> >> http://git.savannah.gnu.org/cgit/emacs.git/log/?qt=author&q=Konstantin
> >> there is no my contribution.
> >>
> >> More over: you didn't even tell me! Wow! This is not good.
> >
> > What do you mean? Your name is in the log.
>
> If you click the link above you won't find it. The only mention of my
> name is in log message as "suggested-by".
If you are looking at partial information, don't be surprised that you
see only some part of what was done and written. Emacs contributors
are generally expected to have a clone of the repository on their
local machine. Don't you have it?
> Which btw sounds like I reported a bug "hey guys, I don't know how
> to code, but static analyzer says this part can be improved, can
> anybody do it for me?".
That's not what it means. "Suggested by" means you suggested a code
change, but I tweaked it enough to feel that you can no longer be held
responsible for whatever mistakes I might have made in the actual code
change I committed. And I explicitly said that this is what I did: "I
pushed a slightly different change along the same lines." There's
also a reference to the bug number, where what you submitted is
clearly visible. The "I don't know how to code" variant would have
been identified as "reported by" instead.
This is all standard practice around here (and in other projects as
well).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 15 Apr 2019 03:40:02 GMT)
Full text and
rfc822 format available.
Message #128 received at 35062 <at> debbugs.gnu.org (full text, mbox):
[[[ To any NSA and FBI agents reading my email: please consider ]]]
[[[ whether defending the US Constitution against all enemies, ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]
> "Suggested by" means you suggested a code
> change, but I tweaked it enough to feel that you can no longer be held
> responsible for whatever mistakes I might have made in the actual code
> change I committed.
I think you're saying that "suggested by" means that the code changes
were suggested by that person -- not merely that the idea was
suggested by per. Is that it?
--
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 15 Apr 2019 06:50:02 GMT)
Full text and
rfc822 format available.
Message #131 received at 35062 <at> debbugs.gnu.org (full text, mbox):
On Вс, Apr 14, 2019 at 23:38, Richard Stallman <rms <at> gnu.org> wrote:
> [[[ To any NSA and FBI agents reading my email: please consider ]]]
> [[[ whether defending the US Constitution against all enemies, ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
> > "Suggested by" means you suggested a code
> > change, but I tweaked it enough to feel that you can no longer be
> held
> > responsible for whatever mistakes I might have made in the actual
> code
> > change I committed.
>
> I think you're saying that "suggested by" means that the code changes
> were suggested by that person -- not merely that the idea was
> suggested by per. Is that it?
That's what you quoted :)
IMO in such situations (i.e. when original changes were commited
without any modification anyway) would be nice to commit the original
patch, and then add up further improvements as 2-nd commit. Ultimately
what makes me sad is that if I'd want to refer to my commits in Emacs
as part of a CV, it's hard to find all suggested-by and authored
commits at the same time, and also that suggested-by sounds kind of
vague to have an influence in CV.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 15 Apr 2019 14:26:02 GMT)
Full text and
rfc822 format available.
Message #134 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> From: Richard Stallman <rms <at> gnu.org>
> Cc: hi-angel <at> yandex.ru, 35062 <at> debbugs.gnu.org
> Date: Sun, 14 Apr 2019 23:38:51 -0400
>
> > "Suggested by" means you suggested a code
> > change, but I tweaked it enough to feel that you can no longer be held
> > responsible for whatever mistakes I might have made in the actual code
> > change I committed.
>
> I think you're saying that "suggested by" means that the code changes
> were suggested by that person -- not merely that the idea was
> suggested by per. Is that it?
It could mean either one, actually.
We have only a small number of attributions that change-log-mode
highlights: "Reported by", "Suggested by", and "Patches by" (each one
with a couple of variants). So "Suggested by" will have to serve both
when someone just describes an idea and when they submit code from
which only the main ideas are taken.
If someone wants to know what exactly was "suggested", they need to
read the relevant discussions. In this case, the log message
references a bug number, and there one can see the actual submission.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 15 Apr 2019 14:33:02 GMT)
Full text and
rfc822 format available.
Message #137 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Mon, 15 Apr 2019 09:49:08 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 35062 <at> debbugs.gnu.org
>
> IMO in such situations (i.e. when original changes were commited
> without any modification anyway) would be nice to commit the original
> patch, and then add up further improvements as 2-nd commit.
That's true, but this is not such a situation: the original changes
were never committed without any modifications.
Sometimes committing the original and then making changes in a
followup is TRT, and sometimes it isn't; it's a judgment call. In
general, the decision depends on the percentage of the original
submission that the committer would like to change, and also on the
overall volume of the original submission. In this case, the original
patch was relatively small, and I modified it in relatively
significant ways. So it made little sense to commit something that
would be immediately modified in significant ways, it would just be
extra work for no good reason.
> Ultimately what makes me sad is that if I'd want to refer to my
> commits in Emacs as part of a CV, it's hard to find all suggested-by
> and authored commits at the same time, and also that suggested-by
> sounds kind of vague to have an influence in CV.
You can always use "git log --grep" to find references to your
contributions in the log messages. And the log message includes a
reference to the bug number, where you can refer people for your
actual contribution.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 15 Apr 2019 15:02:02 GMT)
Full text and
rfc822 format available.
Message #140 received at 35062 <at> debbugs.gnu.org (full text, mbox):
On Пн, Apr 15, 2019 at 17:32, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> Date: Mon, 15 Apr 2019 09:49:08 +0300
>> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, 35062 <at> debbugs.gnu.org
>>
>> IMO in such situations (i.e. when original changes were commited
>> without any modification anyway) would be nice to commit the
>> original
>> patch, and then add up further improvements as 2-nd commit.
>
> That's true, but this is not such a situation: the original changes
> were never committed without any modifications.
Well, given the line my patch modifies has no changes, the only
modification was the commit message. My only mistake was not knowing
that UTF8 is prohibited. But really, it's a 2 symbols text replacement,
me or you could just replace it.
> Sometimes committing the original and then making changes in a
> followup is TRT, and sometimes it isn't; it's a judgment call. In
> general, the decision depends on the percentage of the original
> submission that the committer would like to change, and also on the
> overall volume of the original submission. In this case, the original
> patch was relatively small, and I modified it in relatively
> significant ways. So it made little sense to commit something that
> would be immediately modified in significant ways, it would just be
> extra work for no good reason.
Is there an extra work? The changes you added can be commited with α)
git commit --amend -v, or β) git commit -v. You did α, which only
differs from β by a number of characters, that is ironically smaller
in β.
>> Ultimately what makes me sad is that if I'd want to refer to my
>> commits in Emacs as part of a CV, it's hard to find all suggested-by
>> and authored commits at the same time, and also that suggested-by
>> sounds kind of vague to have an influence in CV.
>
> You can always use "git log --grep" to find references to your
> contributions in the log messages. And the log message includes a
> reference to the bug number, where you can refer people for your
> actual contribution.
Who would attach a bunch of commit messages to a CV? It's unreliable
(interviewer gotta check that these indeed are mine), and also they
gonna bloat CV for no reason. A better way would be providing a
web-link to a repository with commits.
----
Sorry, I actually feel embarassed that I discussing a trivial one-liner
patch :D But I can't stop thinking that this could've happened with a
non-one-line or maybe one-line but non-trivial contribution…
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 15 Apr 2019 15:23:01 GMT)
Full text and
rfc822 format available.
Message #143 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Mon, 15 Apr 2019 18:01:35 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: rms <at> gnu.org, 35062 <at> debbugs.gnu.org
>
> > That's true, but this is not such a situation: the original changes
> > were never committed without any modifications.
>
> Well, given the line my patch modifies has no changes, the only
> modification was the commit message. My only mistake was not knowing
> that UTF8 is prohibited. But really, it's a 2 symbols text replacement,
> me or you could just replace it.
No, the log message was not the problem. Look at the code changes,
they were the ones I modified.
> Is there an extra work? The changes you added can be commited with α)
> git commit --amend -v, or β) git commit -v. You did α, which only
> differs from β by a number of characters, that is ironically smaller
> in β.
Yes, this is extra work: it requires one more commit. More steps,
more opportunities to make mistakes, etc. And that's if I'm not
interrupted in the middle of it by something in Real Life, or someone
pushes to upstream in-between, and I need to pull again and perhaps
resolve conflicts. I'd rather avoid such complications for a simple
job like that.
> > You can always use "git log --grep" to find references to your
> > contributions in the log messages. And the log message includes a
> > reference to the bug number, where you can refer people for your
> > actual contribution.
>
> Who would attach a bunch of commit messages to a CV?
I don't know. When I interview software engineers, I don't ask them
for such details, I can look up their contributions myself, given just
the repository URL.
> Sorry, I actually feel embarassed that I discussing a trivial one-liner
> patch :D But I can't stop thinking that this could've happened with a
> non-one-line or maybe one-line but non-trivial contribution…
We are splitting hair, for sure. I think you are unfamiliar with our
procedures, and try too hard to find aspects that you saw elsewhere.
If so, it's a temporary difficulty.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 15 Apr 2019 17:04:02 GMT)
Full text and
rfc822 format available.
Message #146 received at 35062 <at> debbugs.gnu.org (full text, mbox):
В Пн, апр 15, 2019 at 18:21, Eli Zaretskii <eliz <at> gnu.org>
написал:
>> Date: Mon, 15 Apr 2019 18:01:35 +0300
>> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>> Cc: rms <at> gnu.org, 35062 <at> debbugs.gnu.org
>>
>> > That's true, but this is not such a situation: the original
>> changes
>> > were never committed without any modifications.
>>
>> Well, given the line my patch modifies has no changes, the only
>> modification was the commit message. My only mistake was not knowing
>> that UTF8 is prohibited. But really, it's a 2 symbols text
>> replacement,
>> me or you could just replace it.
>
> No, the log message was not the problem. Look at the code changes,
> they were the ones I modified.
The line that I changed is exactly the same. What you did is cleaning
up the function even further, however that didn't touch my
modifications. Your modification is not strictly relevant to my patch.
>> Is there an extra work? The changes you added can be commited with
>> α)
>> git commit --amend -v, or β) git commit -v. You did α, which only
>> differs from β by a number of characters, that is ironically
>> smaller
>> in β.
>
> Yes, this is extra work: it requires one more commit. More steps,
> more opportunities to make mistakes, etc. And that's if I'm not
> interrupted in the middle of it by something in Real Life, or someone
> pushes to upstream in-between, and I need to pull again and perhaps
> resolve conflicts. I'd rather avoid such complications for a simple
> job like that.
Okay, let's compare:
1. Doing extra commit: "git commit"
2. Changing last commit "git commit --amend"
You did 2, do you see how 1 has less symbols?
What other extra steps did you take, writing the commit message? But
since you rewrote mine anyway, this step you would take for either of 1
and 2.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 15 Apr 2019 17:17:04 GMT)
Full text and
rfc822 format available.
Message #149 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Mon, 15 Apr 2019 20:03:05 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: rms <at> gnu.org, 35062 <at> debbugs.gnu.org
>
> > No, the log message was not the problem. Look at the code changes,
> > they were the ones I modified.
>
> The line that I changed is exactly the same. What you did is cleaning
> up the function even further, however that didn't touch my
> modifications. Your modification is not strictly relevant to my patch.
That's your perspective, but not mine. My perspective is that you
changed one line, whereas I changed 3, and by that also changed the
overall logic of the tests.
> Okay, let's compare:
>
> 1. Doing extra commit: "git commit"
> 2. Changing last commit "git commit --amend"
>
> You did 2, do you see how 1 has less symbols?
No I didn't do 2. I just edited the sources and did 1. I never
applied your patch.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 15 Apr 2019 17:30:02 GMT)
Full text and
rfc822 format available.
Message #152 received at 35062 <at> debbugs.gnu.org (full text, mbox):
В Пн, апр 15, 2019 at 20:16, Eli Zaretskii <eliz <at> gnu.org>
написал:
>> Date: Mon, 15 Apr 2019 20:03:05 +0300
>> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>> Cc: rms <at> gnu.org, 35062 <at> debbugs.gnu.org
>>
>> > No, the log message was not the problem. Look at the code
>> changes,
>> > they were the ones I modified.
>>
>> The line that I changed is exactly the same. What you did is
>> cleaning
>> up the function even further, however that didn't touch my
>> modifications. Your modification is not strictly relevant to my
>> patch.
>
> That's your perspective, but not mine. My perspective is that you
> changed one line, whereas I changed 3, and by that also changed the
> overall logic of the tests.
Right, does that contradict to my statement? Your changes and mine are
independent entities that can exist one without the other.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 15 Apr 2019 18:16:02 GMT)
Full text and
rfc822 format available.
Message #155 received at 35062 <at> debbugs.gnu.org (full text, mbox):
[[[ To any NSA and FBI agents reading my email: please consider ]]]
[[[ whether defending the US Constitution against all enemies, ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]
> IMO in such situations (i.e. when original changes were commited
> without any modification anyway) would be nice to commit the original
> patch, and then add up further improvements as 2-nd commit.
That seems ok to me.
--
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 15 Apr 2019 18:22:02 GMT)
Full text and
rfc822 format available.
Message #158 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Mon, 15 Apr 2019 20:29:03 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: rms <at> gnu.org, 35062 <at> debbugs.gnu.org
>
> > That's your perspective, but not mine. My perspective is that you
> > changed one line, whereas I changed 3, and by that also changed the
> > overall logic of the tests.
>
> Right, does that contradict to my statement? Your changes and mine are
> independent entities that can exist one without the other.
In retrospect, yes. But that's just a coincidence, because I produced
the actual patch I pushed by considering the original logic and how I
would like to change it.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Mon, 15 Apr 2019 18:41:02 GMT)
Full text and
rfc822 format available.
Message #161 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> From: Richard Stallman <rms <at> gnu.org>
> Date: Mon, 15 Apr 2019 14:14:48 -0400
> Cc: 35062 <at> debbugs.gnu.org
>
> > IMO in such situations (i.e. when original changes were commited
> > without any modification anyway) would be nice to commit the original
> > patch, and then add up further improvements as 2-nd commit.
>
> That seems ok to me.
I guess you haven't yet read my response where I explained why it
wasn't OK in this case.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Tue, 16 Apr 2019 21:28:02 GMT)
Full text and
rfc822 format available.
Message #164 received at 35062 <at> debbugs.gnu.org (full text, mbox):
[[[ To any NSA and FBI agents reading my email: please consider ]]]
[[[ whether defending the US Constitution against all enemies, ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]
> We have only a small number of attributions that change-log-mode
> highlights: "Reported by", "Suggested by", and "Patches by" (each one
> with a couple of variants).
We can change that, right? So let's decide which ones we would LIKE,
and make change-log-mode handle them.
--
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Wed, 17 Apr 2019 02:41:02 GMT)
Full text and
rfc822 format available.
Message #167 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> From: Richard Stallman <rms <at> gnu.org>
> Cc: hi-angel <at> yandex.ru, 35062 <at> debbugs.gnu.org
> Date: Tue, 16 Apr 2019 17:27:43 -0400
>
> > We have only a small number of attributions that change-log-mode
> > highlights: "Reported by", "Suggested by", and "Patches by" (each one
> > with a couple of variants).
>
> We can change that, right? So let's decide which ones we would LIKE,
> and make change-log-mode handle them.
I see no reason to change this, the existing possibilities are more
than enough. I never had any problems with using them.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Wed, 17 Apr 2019 20:53:01 GMT)
Full text and
rfc822 format available.
Message #170 received at 35062 <at> debbugs.gnu.org (full text, mbox):
[[[ To any NSA and FBI agents reading my email: please consider ]]]
[[[ whether defending the US Constitution against all enemies, ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]
> > We can change that, right? So let's decide which ones we would LIKE,
> > and make change-log-mode handle them.
> I see no reason to change this, the existing possibilities are more
> than enough. I never had any problems with using them.
We had one just now -- a contributor's career could be helped if we
distinguished one additional option. Others may have the same wish.
It won't take much work, so let's be helpful for them.
--
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 20 Apr 2019 00:32:01 GMT)
Full text and
rfc822 format available.
Message #173 received at 35062 <at> debbugs.gnu.org (full text, mbox):
I'd rather not use 'const' on locals. Let me tell you a story as to why.
I once worked with someone who insisted on using the 'register' keyword on every
C local variable that was never taken the address of, on the grounds that this
meant the reader could easily see that the variable could never be modified
indirectly via a pointer and that this made the code easier to read because you
didn't need to worry about aliasing.
I disagreed then, and still disagree. Saying 'register' nearly all the time
clutters up the code, and the cost is not worth the benefit in C. It's pretty
easy for a human reader to determine whether a local variable is taken the
address of somewher in its function. (If it's hard, then write an Elisp function
that will tell you. :-) In hindsight, perhaps C should have been designed so
that 'register' was the default for local variables, and that one needed a
special word to say "watch out! this variable might have its address taken!";
but the ship has sailed.
'const' is like 'register' in this respect. Putting 'const' nearly everywhere
clutters C code. It's pretty easy for a human reader to determine whether a
local variable is modified (if it's hard, write an Elisp function :-). In
hindsight, perhaps C should have been designed so that 'const' was the default
for local variables; but the ship has sailed there too.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 20 Apr 2019 01:10:01 GMT)
Full text and
rfc822 format available.
Message #176 received at 35062 <at> debbugs.gnu.org (full text, mbox):
В Пт, апр 19, 2019 at 17:31, Paul Eggert <eggert <at> cs.ucla.edu>
написал:
> I'd rather not use 'const' on locals. Let me tell you a story as to
> why.
>
> I once worked with someone who insisted on using the 'register'
> keyword on every C local variable that was never taken the address
> of, on the grounds that this meant the reader could easily see that
> the variable could never be modified indirectly via a pointer and
> that this made the code easier to read because you didn't need to
> worry about aliasing.
>
> I disagreed then, and still disagree. Saying 'register' nearly all
> the time clutters up the code, and the cost is not worth the benefit
> in C. It's pretty easy for a human reader to determine whether a
> local variable is taken the address of somewher in its function. (If
> it's hard, then write an Elisp function that will tell you. :-) In
> hindsight, perhaps C should have been designed so that 'register' was
> the default for local variables, and that one needed a special word
> to say "watch out! this variable might have its address taken!"; but
> the ship has sailed.
>
> 'const' is like 'register' in this respect. Putting 'const' nearly
> everywhere clutters C code. It's pretty easy for a human reader to
> determine whether a local variable is modified.
"const" is only similar to "register" in a way of having an additional
word. Sure it's easy for a human to find whether variable was modified,
but this requires to go through the whole function highlighting
variable usages. Now imagine if visibility scope has lots of variables?
This is especially relevant for the old C89 style where every variable
was declared at the beginning of a function.
And btw, current workflow with searching through the body doesn't work
well in vanilla Emacs. While other editors allow you to highlight
matches by putting a caret over a symbol, Emacs requires either
manually making it highlight/unhighlight, or installing a separate
package highlight-symbol.el which is a bit buggy and wasn't updated
since 2016. That is to say, having non-modified things declared with
"const" may help an average Emacs developer.
> (if it's hard, write an Elisp function :-)
But situation in there is similar: declaring a local variable requires
to use (let…), which means that even if you use variable once in the
end of a function, the variable will be visible to everything else. And
it's not like you can can make it immutable either.
> In hindsight, perhaps C should have been designed so that 'const'
> was the default for local variables; but the ship has sailed there
> too.
Yeah… FWIW, there's an "REmacs" effort on rewriting Emacs from C to
Rust, Rust has everything "const" by default. I'm sure emacs-devel
would largely disagree on integrating it because Rust isn't stable yet
(despite being widely deployed) and there's no GCC backend (LLVM only),
which are reasonable points. Still, I'm hoping at some point it will
change :)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 20 Apr 2019 01:19:02 GMT)
Full text and
rfc822 format available.
Message #179 received at 35062 <at> debbugs.gnu.org (full text, mbox):
В Сб, апр 20, 2019 at 04:09, Konstantin Kharlamov
<hi-angel <at> yandex.ru> написал:
>
>
> В Пт, апр 19, 2019 at 17:31, Paul Eggert <eggert <at> cs.ucla.edu>
> написал:
>> I'd rather not use 'const' on locals. Let me tell you a story as to
>> why.
>>
>> I once worked with someone who insisted on using the 'register'
>> keyword on every C local variable that was never taken the address
>> of, on the grounds that this meant the reader could easily see that
>> the variable could never be modified indirectly via a pointer and
>> that this made the code easier to read because you didn't need to
>> worry about aliasing.
>>
>> I disagreed then, and still disagree. Saying 'register' nearly all
>> the time clutters up the code, and the cost is not worth the
>> benefit in C. It's pretty easy for a human reader to determine
>> whether a local variable is taken the address of somewher in its
>> function. (If it's hard, then write an Elisp function that will
>> tell you. :-) In hindsight, perhaps C should have been designed so
>> that 'register' was the default for local variables, and that one
>> needed a special word to say "watch out! this variable might have
>> its address taken!"; but the ship has sailed.
>>
>> 'const' is like 'register' in this respect. Putting 'const' nearly
>> everywhere clutters C code. It's pretty easy for a human reader to
>> determine whether a local variable is modified.
>
> "const" is only similar to "register" in a way of having an
> additional word. Sure it's easy for a human to find whether variable
> was modified, but this requires to go through the whole function
> highlighting variable usages. Now imagine if visibility scope has
> lots of variables? This is especially relevant for the old C89 style
> where every variable was declared at the beginning of a function.
>
> And btw, current workflow with searching through the body doesn't
> work well in vanilla Emacs. While other editors allow you to
> highlight matches by putting a caret over a symbol, Emacs requires
> either manually making it highlight/unhighlight, or installing a
> separate package highlight-symbol.el which is a bit buggy and wasn't
> updated since 2016. That is to say, having non-modified things
> declared with "const" may help an average Emacs developer.
Shameless plug here: I'm using "color-identifiers" package, which
colors variables in different colors. This is an amazing help! Still,
unfortunately a number of immediately clear distinct colors is quite
limited, so I'm still often use highlight-symbol.el functional.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 20 Apr 2019 06:29:01 GMT)
Full text and
rfc822 format available.
Message #182 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Cc: 35062 <at> debbugs.gnu.org
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> Date: Fri, 19 Apr 2019 17:31:38 -0700
>
> 'const' is like 'register' in this respect. Putting 'const' nearly everywhere
> clutters C code. It's pretty easy for a human reader to determine whether a
> local variable is modified (if it's hard, write an Elisp function :-). In
> hindsight, perhaps C should have been designed so that 'const' was the default
> for local variables; but the ship has sailed there too.
Thanks. That was more or less my thinking as well.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 20 Apr 2019 06:55:02 GMT)
Full text and
rfc822 format available.
Message #185 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 20 Apr 2019 04:09:19 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 35062 <at> debbugs.gnu.org
>
> And btw, current workflow with searching through the body doesn't work
> well in vanilla Emacs. While other editors allow you to highlight
> matches by putting a caret over a symbol, Emacs requires either
> manually making it highlight/unhighlight, or installing a separate
> package highlight-symbol.el which is a bit buggy and wasn't updated
> since 2016. That is to say, having non-modified things declared with
> "const" may help an average Emacs developer.
I don't think I follow: Emacs's search commands do highlight the
matches, and we also have symbol-search commands ("M-s _" etc.), so
what exactly is missing?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 20 Apr 2019 10:32:01 GMT)
Full text and
rfc822 format available.
Message #188 received at 35062 <at> debbugs.gnu.org (full text, mbox):
On Сб, Apr 20, 2019 at 09:53, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> Date: Sat, 20 Apr 2019 04:09:19 +0300
>> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, 35062 <at> debbugs.gnu.org
>>
>> And btw, current workflow with searching through the body doesn't
>> work
>> well in vanilla Emacs. While other editors allow you to highlight
>> matches by putting a caret over a symbol, Emacs requires either
>> manually making it highlight/unhighlight, or installing a separate
>> package highlight-symbol.el which is a bit buggy and wasn't updated
>> since 2016. That is to say, having non-modified things declared with
>> "const" may help an average Emacs developer.
>
> I don't think I follow: Emacs's search commands do highlight the
> matches, and we also have symbol-search commands ("M-s _" etc.), so
> what exactly is missing?
See: to any search a word in Emacs you have to either type it manually
or copy-paste it. And then if you get unlucky to have next match
offscreen, the search gonna carry currently visible portion of text
away.
In other editors and IDEs it's implemented instead by selecting a word,
which makes the editor to highlight matches. That's similar to what
highlight-symbol.el is doing: you put a caret over a text, and after a
short timeout (IIRC you can't set timeout to 0 as it introduces lags to
Emacs) it highlights visible matches of the symbol under the caret. And
then, if you want to, you can press a hotkey to lock the highlight.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 20 Apr 2019 11:03:02 GMT)
Full text and
rfc822 format available.
Message #191 received at 35062 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 20 Apr 2019 13:31:44 +0300
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Cc: eggert <at> cs.ucla.edu, 35062 <at> debbugs.gnu.org
>
> On Сб, Apr 20, 2019 at 09:53, Eli Zaretskii <eliz <at> gnu.org> wrote:
> >> Date: Sat, 20 Apr 2019 04:09:19 +0300
> >> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> >> Cc: Eli Zaretskii <eliz <at> gnu.org>, 35062 <at> debbugs.gnu.org
> >>
> > I don't think I follow: Emacs's search commands do highlight the
> > matches, and we also have symbol-search commands ("M-s _" etc.), so
> > what exactly is missing?
>
> See: to any search a word in Emacs you have to either type it manually
> or copy-paste it. And then if you get unlucky to have next match
> offscreen, the search gonna carry currently visible portion of text
> away.
>
> In other editors and IDEs it's implemented instead by selecting a word,
> which makes the editor to highlight matches.
"M-s ." will highlight matches for the symbol at point, without even
requiring you to select that symbol.
> That's similar to what highlight-symbol.el is doing: you put a caret
> over a text, and after a short timeout (IIRC you can't set timeout
> to 0 as it introduces lags to Emacs) it highlights visible matches
> of the symbol under the caret. And then, if you want to, you can
> press a hotkey to lock the highlight.
So you want to avoid typing "M-s .", is that right? But selecting a
symbol will need more keystrokes (except for very short symbols), so
where's the gain in that?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 20 Apr 2019 11:24:01 GMT)
Full text and
rfc822 format available.
Message #194 received at 35062 <at> debbugs.gnu.org (full text, mbox):
On Сб, Apr 20, 2019 at 14:01, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> Date: Sat, 20 Apr 2019 13:31:44 +0300
>> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>> Cc: eggert <at> cs.ucla.edu, 35062 <at> debbugs.gnu.org
>>
>> On Сб, Apr 20, 2019 at 09:53, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> >> Date: Sat, 20 Apr 2019 04:09:19 +0300
>> >> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>> >> Cc: Eli Zaretskii <eliz <at> gnu.org>, 35062 <at> debbugs.gnu.org
>> >>
>> > I don't think I follow: Emacs's search commands do highlight the
>> > matches, and we also have symbol-search commands ("M-s _" etc.),
>> so
>> > what exactly is missing?
>>
>> See: to any search a word in Emacs you have to either type it
>> manually
>> or copy-paste it. And then if you get unlucky to have next match
>> offscreen, the search gonna carry currently visible portion of text
>> away.
>>
>> In other editors and IDEs it's implemented instead by selecting a
>> word,
>> which makes the editor to highlight matches.
>
> "M-s ." will highlight matches for the symbol at point, without even
> requiring you to select that symbol.
Wow, that's cool, I didn't know about (isearch-forward-symbol-at-point).
>> That's similar to what highlight-symbol.el is doing: you put a caret
>> over a text, and after a short timeout (IIRC you can't set timeout
>> to 0 as it introduces lags to Emacs) it highlights visible matches
>> of the symbol under the caret. And then, if you want to, you can
>> press a hotkey to lock the highlight.
>
> So you want to avoid typing "M-s .", is that right? But selecting a
> symbol will need more keystrokes (except for very short symbols), so
> where's the gain in that?
Well, me not knowing about "M-s ." aside, I understand one can't
directly compare having to select a text in other editors with Emacs,
because those are mouse-oriented, and Emacs is keyboard-oriented. The
"Emacsy" solution is implemented in highlight-symbol.el, and it is
quicker than typing "M-s .": you just put a caret over a symbol, and
matches are automatically shown.
I just thought if I can write an analog of highlight-symbol.el by
binding the "M-s ."
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 20 Apr 2019 11:26:02 GMT)
Full text and
rfc822 format available.
Message #197 received at 35062 <at> debbugs.gnu.org (full text, mbox):
On Сб, Apr 20, 2019 at 14:23, Konstantin Kharlamov
<hi-angel <at> yandex.ru> wrote:
>
>
> On Сб, Apr 20, 2019 at 14:01, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>> Date: Sat, 20 Apr 2019 13:31:44 +0300
>>> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>>> Cc: eggert <at> cs.ucla.edu, 35062 <at> debbugs.gnu.org
>>>
>>> On Сб, Apr 20, 2019 at 09:53, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>> >> Date: Sat, 20 Apr 2019 04:09:19 +0300
>>> >> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>>> >> Cc: Eli Zaretskii <eliz <at> gnu.org>, 35062 <at> debbugs.gnu.org
>>> >>
>>> > I don't think I follow: Emacs's search commands do highlight the
>>> > matches, and we also have symbol-search commands ("M-s _" etc.),
>>> so
>>> > what exactly is missing?
>>>
>>> See: to any search a word in Emacs you have to either type it
>>> manually
>>> or copy-paste it. And then if you get unlucky to have next match
>>> offscreen, the search gonna carry currently visible portion of text
>>> away.
>>>
>>> In other editors and IDEs it's implemented instead by selecting a
>>> word,
>>> which makes the editor to highlight matches.
>>
>> "M-s ." will highlight matches for the symbol at point, without even
>> requiring you to select that symbol.
>
> Wow, that's cool, I didn't know about
> (isearch-forward-symbol-at-point).
>
>>> That's similar to what highlight-symbol.el is doing: you put a
>>> caret
>>> over a text, and after a short timeout (IIRC you can't set timeout
>>> to 0 as it introduces lags to Emacs) it highlights visible matches
>>> of the symbol under the caret. And then, if you want to, you can
>>> press a hotkey to lock the highlight.
>>
>> So you want to avoid typing "M-s .", is that right? But selecting a
>> symbol will need more keystrokes (except for very short symbols), so
>> where's the gain in that?
>
> Well, me not knowing about "M-s ." aside, I understand one can't
> directly compare having to select a text in other editors with Emacs,
> because those are mouse-oriented, and Emacs is keyboard-oriented. The
> "Emacsy" solution is implemented in highlight-symbol.el, and it is
> quicker than typing "M-s .": you just put a caret over a symbol, and
> matches are automatically shown.
>
> I just thought if I can write an analog of highlight-symbol.el by
> binding the "M-s ."
…by binding the function to idle-timeout and post that as an answer
here
https://stackoverflow.com/questions/385661/how-to-highlight-all-occurrences-of-a-word-in-an-emacs-buffer
but I figured that this gonna make caret to jump to beginning of a word
every time, so probably no.
(sorry, accidentally pressed Ctrl+enter in mail client instead of
Ctrl+Backspace)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 20 Apr 2019 11:48:01 GMT)
Full text and
rfc822 format available.
Message #200 received at 35062 <at> debbugs.gnu.org (full text, mbox):
On Сб, Apr 20, 2019 at 14:25, Konstantin Kharlamov
<hi-angel <at> yandex.ru> wrote:
>
>
> On Сб, Apr 20, 2019 at 14:23, Konstantin Kharlamov
> <hi-angel <at> yandex.ru> wrote:
>>
>>
>> On Сб, Apr 20, 2019 at 14:01, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>>> Date: Sat, 20 Apr 2019 13:31:44 +0300
>>>> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>>>> Cc: eggert <at> cs.ucla.edu, 35062 <at> debbugs.gnu.org
>>>>
>>>> On Сб, Apr 20, 2019 at 09:53, Eli Zaretskii <eliz <at> gnu.org>
>>>> wrote:
>>>> >> Date: Sat, 20 Apr 2019 04:09:19 +0300
>>>> >> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>>>> >> Cc: Eli Zaretskii <eliz <at> gnu.org>, 35062 <at> debbugs.gnu.org
>>>> >>
>>>> > I don't think I follow: Emacs's search commands do highlight the
>>>> > matches, and we also have symbol-search commands ("M-s _"
>>>> etc.), so
>>>> > what exactly is missing?
>>>>
>>>> See: to any search a word in Emacs you have to either type it
>>>> manually
>>>> or copy-paste it. And then if you get unlucky to have next match
>>>> offscreen, the search gonna carry currently visible portion of
>>>> text
>>>> away.
>>>>
>>>> In other editors and IDEs it's implemented instead by selecting a
>>>> word,
>>>> which makes the editor to highlight matches.
>>>
>>> "M-s ." will highlight matches for the symbol at point, without even
>>> requiring you to select that symbol.
>>
>> Wow, that's cool, I didn't know about
>> (isearch-forward-symbol-at-point).
>>
>>>> That's similar to what highlight-symbol.el is doing: you put a
>>>> caret
>>>> over a text, and after a short timeout (IIRC you can't set timeout
>>>> to 0 as it introduces lags to Emacs) it highlights visible matches
>>>> of the symbol under the caret. And then, if you want to, you can
>>>> press a hotkey to lock the highlight.
>>>
>>> So you want to avoid typing "M-s .", is that right? But selecting a
>>> symbol will need more keystrokes (except for very short symbols), so
>>> where's the gain in that?
>>
>> Well, me not knowing about "M-s ." aside, I understand one can't
>> directly compare having to select a text in other editors with
>> Emacs, because those are mouse-oriented, and Emacs is
>> keyboard-oriented. The "Emacsy" solution is implemented in
>> highlight-symbol.el, and it is quicker than typing "M-s .": you
>> just put a caret over a symbol, and matches are automatically shown.
>>
>> I just thought if I can write an analog of highlight-symbol.el by
>> binding the "M-s ."
>
> …by binding the function to idle-timeout and post that as an answer
> here
> https://stackoverflow.com/questions/385661/how-to-highlight-all-occurrences-of-a-word-in-an-emacs-buffer
> but I figured that this gonna make caret to jump to beginning of a
> word every time, so probably no.
>
> (sorry, accidentally pressed Ctrl+enter in mail client instead of
> Ctrl+Backspace)
>
Btw, I just tried setting (setq highlight-symbol-idle-delay 0) in
highlight-symbol.el, and it doesn't lag for me! This is amazing! I'll
try to keep it for today to see how it goes.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sat, 20 Apr 2019 11:59:02 GMT)
Full text and
rfc822 format available.
Message #203 received at 35062 <at> debbugs.gnu.org (full text, mbox):
On Сб, Apr 20, 2019 at 14:47, Konstantin Kharlamov
<hi-angel <at> yandex.ru> wrote:
>
>
> On Сб, Apr 20, 2019 at 14:25, Konstantin Kharlamov
> <hi-angel <at> yandex.ru> wrote:
>>
>>
>> On Сб, Apr 20, 2019 at 14:23, Konstantin Kharlamov
>> <hi-angel <at> yandex.ru> wrote:
>>>
>>>
>>> On Сб, Apr 20, 2019 at 14:01, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>>>> Date: Sat, 20 Apr 2019 13:31:44 +0300
>>>>> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>>>>> Cc: eggert <at> cs.ucla.edu, 35062 <at> debbugs.gnu.org
>>>>>
>>>>> On Сб, Apr 20, 2019 at 09:53, Eli Zaretskii <eliz <at> gnu.org>
>>>>> wrote:
>>>>> >> Date: Sat, 20 Apr 2019 04:09:19 +0300
>>>>> >> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>>>>> >> Cc: Eli Zaretskii <eliz <at> gnu.org>, 35062 <at> debbugs.gnu.org
>>>>> >>
>>>>> > I don't think I follow: Emacs's search commands do highlight
>>>>> the
>>>>> > matches, and we also have symbol-search commands ("M-s _"
>>>>> etc.), so
>>>>> > what exactly is missing?
>>>>>
>>>>> See: to any search a word in Emacs you have to either type it
>>>>> manually
>>>>> or copy-paste it. And then if you get unlucky to have next match
>>>>> offscreen, the search gonna carry currently visible portion of
>>>>> text
>>>>> away.
>>>>>
>>>>> In other editors and IDEs it's implemented instead by selecting
>>>>> a word,
>>>>> which makes the editor to highlight matches.
>>>>
>>>> "M-s ." will highlight matches for the symbol at point, without
>>>> even
>>>> requiring you to select that symbol.
>>>
>>> Wow, that's cool, I didn't know about
>>> (isearch-forward-symbol-at-point).
>>>
>>>>> That's similar to what highlight-symbol.el is doing: you put a
>>>>> caret
>>>>> over a text, and after a short timeout (IIRC you can't set
>>>>> timeout
>>>>> to 0 as it introduces lags to Emacs) it highlights visible
>>>>> matches
>>>>> of the symbol under the caret. And then, if you want to, you can
>>>>> press a hotkey to lock the highlight.
>>>>
>>>> So you want to avoid typing "M-s .", is that right? But selecting
>>>> a
>>>> symbol will need more keystrokes (except for very short symbols),
>>>> so
>>>> where's the gain in that?
>>>
>>> Well, me not knowing about "M-s ." aside, I understand one can't
>>> directly compare having to select a text in other editors with
>>> Emacs, because those are mouse-oriented, and Emacs is
>>> keyboard-oriented. The "Emacsy" solution is implemented in
>>> highlight-symbol.el, and it is quicker than typing "M-s .": you
>>> just put a caret over a symbol, and matches are automatically
>>> shown.
>>>
>>> I just thought if I can write an analog of highlight-symbol.el by
>>> binding the "M-s ."
>>
>> …by binding the function to idle-timeout and post that as an
>> answer here
>> https://stackoverflow.com/questions/385661/how-to-highlight-all-occurrences-of-a-word-in-an-emacs-buffer
>> but I figured that this gonna make caret to jump to beginning of a
>> word every time, so probably no.
>>
>> (sorry, accidentally pressed Ctrl+enter in mail client instead of
>> Ctrl+Backspace)
>>
> Btw, I just tried setting (setq highlight-symbol-idle-delay 0) in
> highlight-symbol.el, and it doesn't lag for me! This is amazing! I'll
> try to keep it for today to see how it goes.
>
Ah, never mind, typing the text is impossible.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 23 Jun 2019 18:08:02 GMT)
Full text and
rfc822 format available.
Message #206 received at 35062 <at> debbugs.gnu.org (full text, mbox):
Konstantin Kharlamov <hi-angel <at> yandex.ru> writes:
> Per discussion on emacs-devel, this topic is created so that following
> patches wouldn't create separate reports.
I've skimmed this very long bug report now, and it's unclear to me
whether there's any remaining work to be done in this bug report. A
number of patches were posted, and a number were applied, but I'm not
sure whether all that should have been applied were applied?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#35062; Package
emacs.
(Sun, 23 Jun 2019 18:35:02 GMT)
Full text and
rfc822 format available.
Message #209 received at 35062 <at> debbugs.gnu.org (full text, mbox):
Thanks. IIRC everything here was addressed, so it can be closed.
23.06.2019, 21:07, "Lars Ingebrigtsen" <larsi <at> gnus.org>:
> Konstantin Kharlamov <hi-angel <at> yandex.ru> writes:
>
>> Per discussion on emacs-devel, this topic is created so that following
>> patches wouldn't create separate reports.
>
> I've skimmed this very long bug report now, and it's unclear to me
> whether there's any remaining work to be done in this bug report. A
> number of patches were posted, and a number were applied, but I'm not
> sure whether all that should have been applied were applied?
>
> --
> (domestic pets only, the antidote for overdose, milk.)
> bloggy blog: http://lars.ingebrigtsen.no
Added tag(s) fixed.
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org.
(Sun, 23 Jun 2019 18:38:02 GMT)
Full text and
rfc822 format available.
bug marked as fixed in version 27.1, send any further explanations to
35062 <at> debbugs.gnu.org and Konstantin Kharlamov <hi-angel <at> yandex.ru>
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org.
(Sun, 23 Jun 2019 18:38:02 GMT)
Full text and
rfc822 format available.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Mon, 22 Jul 2019 11:24:10 GMT)
Full text and
rfc822 format available.
This bug report was last modified 4 years and 273 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.