Skip to content

Commit 8c2d237

Browse files
committed
clipmenud: Make partial merge timing tighter
Use CLOCK_MONOTONIC and compare the actual elapsed time when deciding whether a new clip is close enough to count as a partial merge. This is necessary because the old time(NULL) check only has second granularity. In practice that gives partial_merge_secs extra slack around second boundaries, so a setting of 2 could still merge clips that arrived more than 2 seconds later. Maybe that doesn't matter for users, but it does matter for testing flakiness.
1 parent 697964e commit 8c2d237

2 files changed

Lines changed: 37 additions & 6 deletions

File tree

src/clipmenud.c

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ static struct clip_text last_text[CM_SEL_MAX] = {
6262
{NULL, CLIP_TEXT_SOURCE_MALLOC},
6363
{NULL, CLIP_TEXT_SOURCE_MALLOC},
6464
};
65-
static time_t last_text_time[CM_SEL_MAX];
65+
static struct timespec last_text_time[CM_SEL_MAX];
6666

6767
static void free_clip_text(struct clip_text *ct) {
6868
expect(ct->source != CLIP_TEXT_SOURCE_INVALID);
@@ -79,6 +79,25 @@ static void free_clip_text(struct clip_text *ct) {
7979
ct->source = CLIP_TEXT_SOURCE_INVALID;
8080
}
8181

82+
static struct timespec get_monotonic_time(void) {
83+
struct timespec ts;
84+
expect(clock_gettime(CLOCK_MONOTONIC, &ts) == 0);
85+
return ts;
86+
}
87+
88+
static bool within_partial_merge_window(struct timespec current_time,
89+
struct timespec last_time) {
90+
time_t elapsed_secs = current_time.tv_sec - last_time.tv_sec;
91+
92+
if (elapsed_secs < cfg.partial_merge_secs) {
93+
return true;
94+
}
95+
if (elapsed_secs > cfg.partial_merge_secs) {
96+
return false;
97+
}
98+
return current_time.tv_nsec <= last_time.tv_nsec;
99+
}
100+
82101
// cppcheck-suppress [constParameterCallback,unmatchedSuppression]
83102
static Bool is_timestamp_event(Display *display _unused_, XEvent *event,
84103
XPointer arg) {
@@ -351,11 +370,11 @@ static void maybe_trim(void) {
351370
*/
352371
static uint64_t store_clip(enum selection_type sel, struct clip_text *ct) {
353372
dbg("Clipboard text is considered salient, storing\n");
354-
time_t current_time = time(NULL);
373+
struct timespec current_time = get_monotonic_time();
355374
uint64_t hash;
356375

357376
if (cfg.partial_merge_secs > 0 && last_text[sel].data &&
358-
difftime(current_time, last_text_time[sel]) <= cfg.partial_merge_secs &&
377+
within_partial_merge_window(current_time, last_text_time[sel]) &&
359378
is_possible_partial(last_text[sel].data, ct->data)) {
360379
dbg("Possible partial of last clip on %s, replacing\n",
361380
cfg.selections[sel].name);

tests/x_integration_tests

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,20 @@ check_nr_clips() {
106106
(( $(wc -l < "$l_out") == "${1?}" ))
107107
}
108108

109+
wait_nr_clips() {
110+
local want=${1?}
111+
local deadline=$((SECONDS + 5))
112+
113+
while (( SECONDS < deadline )); do
114+
if check_nr_clips "$want"; then
115+
return 0
116+
fi
117+
sleep 0.1
118+
done
119+
120+
check_nr_clips "$want"
121+
}
122+
109123
if ! (( USE_CURRENT_DISPLAY )); then
110124
export DISPLAY=:1911
111125
Xvfb "$DISPLAY" &
@@ -282,9 +296,7 @@ check_nr_clips 5
282296

283297
# Large selection (INCR)
284298
printf '%.0sa' {1..4001} | xsel -b
285-
long_settle
286-
287-
check_nr_clips 6
299+
wait_nr_clips 6
288300

289301
# After large selection is stored, put something small that's a possible #
290302
# partial (non-INCR)

0 commit comments

Comments
 (0)