Eli Zaretskii writes: >> From: Kévin Le Gouguec >> Cc: gregory@heytings.org, 60841@debbugs.gnu.org >> Date: Sat, 28 Jan 2023 18:45:07 +0100 >> >> diff-mode.el (re. smerge-mode.el) can indeed be fooled into thinking >> diff-changed (re. smerge-refined-changed) differs-from-default, if one >> "shoots their own foot", for example, setting… >> >> * :extend t: fixed by this patch ✔️ >> * :stipple nil: foot blown with or without the patch 🤷 >> * :inherit 'default: foot blown with or without the patch 🤷 >> >> Problem with :stipple nil and :inherit 'default explained in [1]. >> indicate-copied-region will become affected if the current patch goes >> in. > > I wouldn't be bothered by :stipple whose value is nil. Why would > someone do such a thing, except when the face is meant to be merged > with other faces (which are expected to have non-nil :stipple > attributes)? The only place where this "matters" is in replace.el, AFAICT: 1. Customize list-matching-lines-prefix-face to 'default, 2. (face-differs-from-default-p list-matching-lines-prefix-face) ↦ :stipple 3. Luckily this does not impact replace-el:occur-1, since this check… (if (face-differs-from-default-p list-matching-lines-prefix-face) list-matching-lines-prefix-face) … is redundant AFAIU: the goal is to _not_ apply list-matching-lines-prefix-face if it is close to 'default, and instead… leave the default face? So why bother checking? CC'ing Juri, who installed this check in bug#14017, for comment in case I missed something. (Hi Juri 👋 For context, the current report is about refining indicate-copied-region to avoid spuriously considering the region as "unhighlighted" and triggering the point-mark swap; see attached patches. I believe this aspect of the report might also be of interest to you, given your participation in bug#42865) >> > Alternatively, we could add a user option to make the swap >> > unconditional, because maybe some users would prefer that to splitting >> > hair in this case. Then we could stop worrying about all those fine >> > differences. >> >> Should I cook up a user option to unconditionally do the swap before we >> apply the attached? Otherwise we may disgruntle trunk users who >> actually liked the behaviour I reported in the OP (swapping regardless >> of whether region stands out). > > I guess adding such an option would be a good precaution, indeed. OK, two tentative patches attached, because I don't know which makes for the better UX: ▼ Copy Region Inhibit Blink: Choice: (*) region-stands-out-p Whether the region can be distinguished visually. More ( ) always Always inhibit: never blink point and mark. ( ) ignore Never inhibit: always blink point and mark. ( ) Other predicate function.: ignore State : STANDARD. Whether we should refrain from blinking the cursor after a copy. ▼ When this condition holds, ‘kill-ring-save’ will not blink the cursor between point and mark to denote the copied region. Groups: Killing ⁂ VS ⁂ ▼ Copy Region Blink Predicate: Choice: (*) region-indistinguishable-p Whether the current region is not denoted visually. ▼ This holds when the region is inactive, or when the ‘region’ face cannot be distinguished from the ‘default’ face. ( ) always Always blink point and mark. ( ) ignore Never blink point and mark. ( ) Other predicate function.: ignore State : STANDARD. Whether the cursor must be blinked after a copy. ▼ When this condition holds, and the copied region fits in the current window, ‘kill-ring-save’ will blink the cursor between point and mark for ‘copy-region-blink-delay’ seconds. Groups: Killing inhibit-vs-predicate.diff shows a comparison of both patches once applied. Boldly marked the new options with "---" in NEWS, because copy-region-blink-delay is not documented in the manual either, but let me know if that should be remedied. Thank you for your time.