Improved keybind functionality, help menu, index prune speed.
Some checks failed
CI / linux (push) Successful in 14m54s
CI / windows-cross (push) Failing after 6m11s
CI / release (push) Has been skipped

This commit is contained in:
Jeremy Karst 2026-09-05 01:55:59 -04:00
parent fe832109d7
commit 488038c080
30 changed files with 2949 additions and 209 deletions

View file

@ -173,7 +173,7 @@ tutorial_seen = false
[search] [search]
# Start with the fuzzy passes enabled. # Start with the fuzzy passes enabled.
fuzzy_default = false fuzzy_default = true
# Ceiling on the fuzzy stages' typo budget. The allowance grows with the # Ceiling on the fuzzy stages' typo budget. The allowance grows with the
# search term, one edit per three characters, up to this value, so 2 # search term, one edit per three characters, up to this value, so 2
# means "1 edit for 3-5 character terms, 2 for anything longer". 0 turns # means "1 edit for 3-5 character terms, 2 for anything longer". 0 turns

File diff suppressed because it is too large Load diff

View file

@ -23,11 +23,24 @@
//! page read the rows, decide nothing //! page read the rows, decide nothing
//! +cover ...and run the scope test per row //! +cover ...and run the scope test per row
//! +files ...and delete the doomed `files` rows //! +files ...and delete the doomed `files` rows
//! +fts(all) ...and tombstone every doomed id <- ships today //! +fts(all) ...and tombstone every doomed id
//! +fts(done) ...tombstoning only ids that can have an FTS row //! +fts(done) ...tombstoning only ids that can have an FTS row
//! +subtree ...range-deleting a doomed directory instead of paging it //! +subtree ...range-deleting a doomed directory instead of paging it
//! ``` //! ```
//! //!
//! **No stage is what ships any more.** `scope::advance` tombstones only the
//! ids that can hold a posting (`+fts(done)`), commits per slice rather than
//! per page, and turns FTS5's delete-merging off for the pass
//! (`file_handling::fts_begin_tombstone_burst`), so the `live` row below lands
//! under every stage rather than on one of them. The stages remain the
//! decomposition — they say where the time is — and `live` says what the sum
//! of the shipped decisions costs.
//!
//! Two of them were measured and **not** adopted, which is why they are still
//! here: `+subtree` bought nothing over `+fts(done)` (247 ms against 243 on the
//! 40k corpus, with all 8,080 doomed rows genuinely skipping the page loop), and
//! raising the page cache moved misses twelvefold while barely moving the clock.
//!
//! **Read the `commit` column, not `fts`.** FTS5 buffers a contentless delete //! **Read the `commit` column, not `fts`.** FTS5 buffers a contentless delete
//! in memory and writes the tombstone pages when the transaction is flushed, so //! in memory and writes the tombstone pages when the transaction is flushed, so
//! the `DELETE` statement itself times as nearly free and the cost lands in the //! the `DELETE` statement itself times as nearly free and the cost lands in the
@ -824,7 +837,8 @@ fn main() {
// The reference number every stage above is decomposing: the real // The reference number every stage above is decomposing: the real
// `scope::advance`, driven to completion the way the coordinator drives // `scope::advance`, driven to completion the way the coordinator drives
// it. It must land on `+fts(all)`. // it. It lands *under* every stage — see the header for which decisions
// put it there.
{ {
let arm = clone_arm(&master, &format!("prune-{}-live", label)); let arm = clone_arm(&master, &format!("prune-{}-live", label));
let mut conn = open(&arm); let mut conn = open(&arm);

View file

@ -251,7 +251,7 @@ impl Default for ProcessingConfig {
impl Default for SearchConfig { impl Default for SearchConfig {
fn default() -> Self { fn default() -> Self {
SearchConfig { SearchConfig {
fuzzy_default: false, fuzzy_default: true,
fuzzy_max_edits: 2, fuzzy_max_edits: 2,
display_limit: 1000, display_limit: 1000,
results_per_page: 100, results_per_page: 100,

View file

@ -837,7 +837,7 @@ fn newer_fields_round_trip_and_default_when_absent() {
fs::write( fs::write(
&path, &path,
"[paths]\nindexing_paths=[\"/x\"]\ndatabase_path=\"db.sqlite\"\n\ "[paths]\nindexing_paths=[\"/x\"]\ndatabase_path=\"db.sqlite\"\n\
[ui]\nscale=1.25\n[search]\nfuzzy_default=true\ndisplay_limit=250\n", [ui]\nscale=1.25\n[search]\nfuzzy_default=false\ndisplay_limit=250\n",
) )
.unwrap(); .unwrap();
let cfg = Config::load_from(&path).unwrap(); let cfg = Config::load_from(&path).unwrap();
@ -850,7 +850,8 @@ fn newer_fields_round_trip_and_default_when_absent() {
assert_eq!(cfg.ui.color_scheme, "dark"); assert_eq!(cfg.ui.color_scheme, "dark");
assert_eq!(cfg.search.fuzzy_max_edits, 2); assert_eq!(cfg.search.fuzzy_max_edits, 2);
assert_eq!(cfg.ui.scale, 1.25, "existing ui keys still parse"); assert_eq!(cfg.ui.scale, 1.25, "existing ui keys still parse");
assert!(cfg.search.fuzzy_default, "existing search keys still parse"); // `false` is the non-default value, so this only passes if the key parsed.
assert!(!cfg.search.fuzzy_default, "existing search keys still parse");
assert_eq!(cfg.search.display_limit, 250); assert_eq!(cfg.search.display_limit, 250);
// A value nobody recognises is not a broken config file. // A value nobody recognises is not a broken config file.

View file

@ -318,7 +318,23 @@ pub fn raw_text_len(blob: &[u8]) -> Option<u64> {
} }
/// Mark a file's content extraction as failed. Keeps the basic row in place. /// Mark a file's content extraction as failed. Keeps the basic row in place.
///
/// Clears any posting and stored body first, which matters twice over. It is
/// what a re-extraction that fails *owes* the reader: the text that is there
/// came out of an earlier version of a file that has since changed, so leaving
/// it serves hits for content the file no longer has. And it is what makes
/// "a `searchabletext` row exists exactly when `content_state` is
/// `STATE_DONE`" true of every transition rather than of most of them —
/// an equivalence `count_root` reports from and `delete_files_matching` now
/// narrows on, so a transition that quietly broke it would leave postings
/// behind for files that are no longer indexed.
///
/// Every path that reaches here today is already re-extracting a row it has
/// just reset (`update_file_basic`) or that was born pending, so the two
/// deletes are normally no-ops; extraction failures are rare enough that
/// paying for the guarantee is not worth measuring.
pub fn set_content_failed(tx: &Transaction<'_>, file_id: i64, reason: &str) -> Result<(), String> { pub fn set_content_failed(tx: &Transaction<'_>, file_id: i64, reason: &str) -> Result<(), String> {
remove_content_for_id(tx, file_id)?;
let now = crate::log::now_unix() as i64; let now = crate::log::now_unix() as i64;
exec( exec(
tx, tx,
@ -337,7 +353,15 @@ pub fn set_content_failed(tx: &Transaction<'_>, file_id: i64, reason: &str) -> R
/// Mark content extraction as not applicable; the row still serves filename /// Mark content extraction as not applicable; the row still serves filename
/// search. /// search.
///
/// Clears any posting and stored body first, for the reasons spelled out on
/// [`set_content_failed`] — a row arrives here because its content should no
/// longer be searchable, so leaving the old text behind contradicts the very
/// transition. Every caller already cleared first or had nothing to clear, so
/// this changes no behaviour; what it changes is that the invariant no longer
/// depends on all of them remembering.
pub fn set_content_na(tx: &Transaction<'_>, file_id: i64) -> Result<(), String> { pub fn set_content_na(tx: &Transaction<'_>, file_id: i64) -> Result<(), String> {
remove_content_for_id(tx, file_id)?;
set_state_clearing_failure(tx, file_id, STATE_NA, "update NA") set_state_clearing_failure(tx, file_id, STATE_NA, "update NA")
} }
@ -380,6 +404,16 @@ pub fn delete_subtree(tx: &Transaction<'_>, lo: &str, hi: &str) -> Result<usize,
/// and reconcile's `orphans()` sweep. `searchabletext` cannot cascade — an /// and reconcile's `orphans()` sweep. `searchabletext` cannot cascade — an
/// FTS5 virtual table takes no foreign key — so its contentless delete must /// FTS5 virtual table takes no foreign key — so its contentless delete must
/// stay explicit. /// stay explicit.
///
/// **Deliberately *not* narrowed to `content_state = STATE_DONE`**, though the
/// rows it would exclude are provably the ones with nothing to tombstone (see
/// [`delete_ids`], which does narrow). The two are not the same trade: this one
/// works from a range rather than a list of decided rows, so reading
/// `content_state` costs a `files` row fetch per candidate — the very rows the
/// `DELETE` below is about to fetch anyway, but a second traversal of them all
/// the same. That is a certain cost against an uncertain saving, and the saving
/// it buys is one `%_docsize` seek into a b-tree far smaller than `files`.
/// `examples/pruneprobe.rs` prices the range form; narrow this when it says to.
fn delete_files_matching( fn delete_files_matching(
tx: &Transaction<'_>, tx: &Transaction<'_>,
files_where: &str, files_where: &str,
@ -437,18 +471,42 @@ fn placeholders(n: usize) -> String {
/// Delete the given file ids and everything keyed to them. Returns how many /// Delete the given file ids and everything keyed to them. Returns how many
/// `files` rows went. Dependent tables: see `delete_files_matching`. /// `files` rows went. Dependent tables: see `delete_files_matching`.
pub fn delete_ids(tx: &Transaction<'_>, ids: &[i64]) -> Result<usize, String> { ///
let mut removed = 0; /// `with_postings` is the subset of `ids` whose `content_state` was
for chunk in ids.chunks(DELETE_IDS_CHUNK) { /// `STATE_DONE`, and so the only ones FTS5 can have anything to tombstone for —
let placeholders = placeholders(chunk.len()); /// `repo_tests::leaving_done_always_takes_the_posting_with_it` is what makes
/// that true of every transition. Handing FTS5 the rest is not free: each is a
/// `%_docsize` seek to discover an absence, and on an index where most rows
/// carry no text that is most of the list.
///
/// Taking it as a second argument rather than deriving it here is the point:
/// the caller decided these rows from a page it had already read, so it holds
/// `content_state` for nothing, where a `SELECT` back out of `files` would cost
/// a row fetch each (see `delete_files_matching`, which for that reason does
/// not narrow).
///
/// It must be a subset: an id left out keeps its posting after its `files` row
/// is gone, which surfaces as a hit for a file that is no longer indexed.
pub fn delete_ids(
tx: &Transaction<'_>,
ids: &[i64],
with_postings: &[i64],
) -> Result<usize, String> {
for chunk in with_postings.chunks(DELETE_IDS_CHUNK) {
let sql = format!( let sql = format!(
"DELETE FROM searchabletext WHERE rowid IN ({})", "DELETE FROM searchabletext WHERE rowid IN ({})",
placeholders placeholders(chunk.len())
); );
exec(tx, &sql, params_from_iter(chunk.iter()), || { exec(tx, &sql, params_from_iter(chunk.iter()), || {
format!("delete searchabletext for {} ids", chunk.len()) format!("delete searchabletext for {} ids", chunk.len())
})?; })?;
let sql = format!("DELETE FROM files WHERE id IN ({})", placeholders); }
let mut removed = 0;
for chunk in ids.chunks(DELETE_IDS_CHUNK) {
let sql = format!(
"DELETE FROM files WHERE id IN ({})",
placeholders(chunk.len())
);
removed += exec(tx, &sql, params_from_iter(chunk.iter()), || { removed += exec(tx, &sql, params_from_iter(chunk.iter()), || {
format!("delete {} file rows", chunk.len()) format!("delete {} file rows", chunk.len())
})?; })?;
@ -456,6 +514,77 @@ pub fn delete_ids(tx: &Transaction<'_>, ids: &[i64]) -> Result<usize, String> {
Ok(removed) Ok(removed)
} }
/// Set `content_state` on many rows at once, leaving everything else alone.
///
/// The batch form of the `UPDATE` inside [`set_state_clearing_failure`], and
/// deliberately *without* its `failed_files` sweep: only a `STATE_FAILED` row
/// can hold such a record, so a caller that knows the stored states can clear
/// the few that need it with [`clear_failed_for_ids`] instead of paying a
/// delete per row. A caller that does not know them must call the per-row
/// helpers, which cannot get this wrong.
pub fn set_content_state(tx: &Transaction<'_>, ids: &[i64], state: i64) -> Result<usize, String> {
let mut changed = 0;
for chunk in ids.chunks(DELETE_IDS_CHUNK) {
let sql = format!(
"UPDATE files SET content_state = ?1 WHERE id IN ({})",
placeholders(chunk.len())
);
let params = params_from_iter(
std::iter::once(&state as &dyn rusqlite::ToSql)
.chain(chunk.iter().map(|id| id as &dyn rusqlite::ToSql)),
);
changed += exec(tx, &sql, params, || {
format!("set content_state {} on {} rows", state, chunk.len())
})?;
}
Ok(changed)
}
/// Drop the FTS posting and the stored body of many rows at once, leaving
/// their `files` rows in place: the batch form of [`remove_content_for_id`].
///
/// Pass only ids whose stored `content_state` was `STATE_DONE`; the others
/// have neither, and asking is what costs (see `delete_files_matching`).
pub fn clear_content_for_ids(tx: &Transaction<'_>, ids: &[i64]) -> Result<(), String> {
for chunk in ids.chunks(DELETE_IDS_CHUNK) {
let list = placeholders(chunk.len());
for (what, sql) in [
(
"searchabletext",
format!("DELETE FROM searchabletext WHERE rowid IN ({})", list),
),
(
"documents_text",
format!("DELETE FROM documents_text WHERE file_id IN ({})", list),
),
] {
exec(tx, &sql, params_from_iter(chunk.iter()), || {
format!("clear {} for {} ids", what, chunk.len())
})?;
}
}
Ok(())
}
/// Forget the failure records of many rows at once. `list-failed` reads
/// `failed_files` directly, so a stale entry keeps reporting a file broken —
/// this is the batch half of what [`set_state_clearing_failure`] does per row.
///
/// Pass only ids whose stored `content_state` was `STATE_FAILED`: nothing else
/// can hold a record here.
pub fn clear_failed_for_ids(tx: &Transaction<'_>, ids: &[i64]) -> Result<(), String> {
for chunk in ids.chunks(DELETE_IDS_CHUNK) {
let sql = format!(
"DELETE FROM failed_files WHERE file_id IN ({})",
placeholders(chunk.len())
);
exec(tx, &sql, params_from_iter(chunk.iter()), || {
format!("clear failed_files for {} ids", chunk.len())
})?;
}
Ok(())
}
/// Every indexed file directly inside `parent`, as `name -> mtime`. `parent` /// Every indexed file directly inside `parent`, as `name -> mtime`. `parent`
/// must be in stored spelling — trailing separator and all; build it with /// must be in stored spelling — trailing separator and all; build it with
/// [`crate::file_handling::dir_to_db_parent`]. /// [`crate::file_handling::dir_to_db_parent`].

View file

@ -527,9 +527,12 @@ fn delete_ids_clears_every_dependent_table() {
} }
let doomed = vec![ids["/t/b.log"], ids["/t/deep/c.log"]]; let doomed = vec![ids["/t/b.log"], ids["/t/deep/c.log"]];
// `c.log` is FAILED and so holds no posting — the distinction the second
// argument draws, and the whole reason it is a separate list.
let with_postings = vec![ids["/t/b.log"]];
let removed = { let removed = {
let tx = conn.transaction().unwrap(); let tx = conn.transaction().unwrap();
let n = delete_ids(&tx, &doomed).unwrap(); let n = delete_ids(&tx, &doomed, &with_postings).unwrap();
tx.commit().unwrap(); tx.commit().unwrap();
n n
}; };
@ -553,7 +556,7 @@ fn delete_ids_clears_every_dependent_table() {
// Empty input is a no-op, not a statement with an empty `IN ()`. // Empty input is a no-op, not a statement with an empty `IN ()`.
let tx = conn.transaction().unwrap(); let tx = conn.transaction().unwrap();
assert_eq!(delete_ids(&tx, &[]).unwrap(), 0); assert_eq!(delete_ids(&tx, &[], &[]).unwrap(), 0);
tx.commit().unwrap(); tx.commit().unwrap();
} }
@ -572,7 +575,8 @@ fn delete_ids_spans_chunk_boundaries() {
let keep = all.pop().unwrap(); let keep = all.pop().unwrap();
let removed = { let removed = {
let tx = conn.transaction().unwrap(); let tx = conn.transaction().unwrap();
let n = delete_ids(&tx, &all).unwrap(); // `seeded` leaves every row DONE, so both lists span the boundary.
let n = delete_ids(&tx, &all, &all).unwrap();
tx.commit().unwrap(); tx.commit().unwrap();
n n
}; };
@ -1199,6 +1203,74 @@ fn fts_rows(conn: &Connection) -> i64 {
.unwrap() .unwrap()
} }
/// Every transition *out* of `STATE_DONE` takes the posting and the stored
/// body with it, so "a `searchabletext` row exists exactly when `content_state`
/// is `STATE_DONE`" holds however a row got where it is.
///
/// `count_root_counts_the_fts_rows_it_says_it_does` below checks the same
/// equivalence over rows that were *never* DONE, which is the easy half — it
/// passes even for a transition that leaves a stale posting behind. This is the
/// half that does not, and `delete_files_matching` narrows its tombstone
/// statement on the strength of it: a row that kept a posting past `DONE` would
/// keep it past deletion too, and answer searches for a file that is gone.
#[test]
fn leaving_done_always_takes_the_posting_with_it() {
let (_dir, p) = tmp_path();
let mut conn = open_or_recreate(p.to_str().unwrap(), "trigram").unwrap();
// Each row starts DONE — with a posting and a stored body — and then takes
// one of the four ways out.
let leave: [(&str, fn(&rusqlite::Transaction<'_>, i64)); 4] = [
("/t/failed.txt", |tx, id| {
set_content_failed(tx, id, "bad parse").unwrap()
}),
("/t/na.txt", |tx, id| set_content_na(tx, id).unwrap()),
("/t/pending.txt", |tx, id| {
reset_content_pending(tx, id).unwrap()
}),
("/t/rewritten.txt", |tx, id| {
// The shape a changed file takes: metadata updated in place.
update_file_basic(
tx,
&NewFile {
name: "rewritten.txt",
parent: "/t/",
size: 99,
mtime: 99,
mime: Some("text/plain"),
ftype: FileType::TEXT,
hash: None,
needs_content: true,
},
)
.unwrap()
.expect("the row is there");
let _ = id;
}),
];
for (path, transition) in leave {
let tx = conn.transaction().unwrap();
let id = insert_at(&tx, path, true);
set_content_done(&tx, id, "body text", zstd_of("body text").as_deref()).unwrap();
transition(&tx, id);
tx.commit().unwrap();
}
let count = |sql: &str| -> i64 { conn.query_row(sql, [], |r| r.get(0)).unwrap() };
assert_eq!(
count("SELECT COUNT(*) FROM files WHERE content_state = 1"),
0,
"every row left DONE"
);
assert_eq!(fts_rows(&conn), 0, "and none of them kept its posting");
assert_eq!(
count("SELECT COUNT(*) FROM documents_text"),
0,
"nor its stored body, which would outlive the file it was read from"
);
}
/// The premise of `count_root`: `content_state = DONE` exactly when a /// The premise of `count_root`: `content_state = DONE` exactly when a
/// `searchabletext` row exists. Pinned against the FTS table itself, because /// `searchabletext` row exists. Pinned against the FTS table itself, because
/// the equivalence is what breaks if a transition writes one without the /// the equivalence is what breaks if a transition writes one without the
@ -1348,12 +1420,16 @@ fn the_parent_scan_reaches_the_roots_own_directory() {
fn deleting_a_file_row_cascades_the_fk_tables() { fn deleting_a_file_row_cascades_the_fk_tables() {
let (_dir, path) = tmp_path(); let (_dir, path) = tmp_path();
let mut conn = open_or_recreate(path.to_str().unwrap(), "trigram").unwrap(); let mut conn = open_or_recreate(path.to_str().unwrap(), "trigram").unwrap();
let ids = seeded(&mut conn, &["/casc/a.txt", "/casc/b.txt"]); seeded(&mut conn, &["/casc/a.txt", "/casc/b.txt"]);
let count = let count =
|conn: &Connection, sql: &str| -> i64 { conn.query_row(sql, [], |r| r.get(0)).unwrap() }; |conn: &Connection, sql: &str| -> i64 { conn.query_row(sql, [], |r| r.get(0)).unwrap() };
{ {
// The failure goes on a row that never extracted, which is the only
// way a run reaches `set_content_failed` — and keeps the two searchable
// rows searchable, so the FTS figures below stay about the cascade.
let tx = conn.transaction().unwrap(); let tx = conn.transaction().unwrap();
set_content_failed(&tx, ids["/casc/b.txt"], "boom").unwrap(); let never = insert_at(&tx, "/casc/c.bin", true);
set_content_failed(&tx, never, "boom").unwrap();
tx.commit().unwrap(); tx.commit().unwrap();
} }
assert_eq!(count(&conn, "SELECT COUNT(*) FROM documents_text"), 2); assert_eq!(count(&conn, "SELECT COUNT(*) FROM documents_text"), 2);

View file

@ -27,7 +27,8 @@ pub(crate) use paths::{
}; };
pub use records::{ pub use records::{
classify_by_mtime, classify_for_indexing, content_extractable, decide_content, classify_by_mtime, classify_for_indexing, content_extractable, decide_content,
extract_and_store, fts_begin_bulk_write, fts_finalize_after_text_indexing, fts_set_automerge, extract_and_store, fts_begin_bulk_write, fts_begin_tombstone_burst, fts_end_tombstone_burst,
fts_finalize_after_text_indexing, fts_set_automerge, FTS_DELETEMERGE,
get_file_hash, hash_failure_counts, outcome_body, prepare_file_record, get_file_hash, hash_failure_counts, outcome_body, prepare_file_record,
prepare_file_record_from_path, reset_run_warnings, store_content_outcome, ContentOutcome, prepare_file_record_from_path, reset_run_warnings, store_content_outcome, ContentOutcome,
DirRows, FileIndexAction, OwnedNewFile, DirRows, FileIndexAction, OwnedNewFile,

View file

@ -116,6 +116,147 @@ pub fn fts_set_automerge(conn: &Connection, segments: u8) {
/// smaller, more build-stable index for no cost; 64 measured identically. /// smaller, more build-stable index for no cost; 64 measured identically.
const WRITE_CRISISMERGE: u8 = 32; const WRITE_CRISISMERGE: u8 = 32;
/// Percentage of a level's entries that must be tombstones before FTS5 rewrites
/// the level to reclaim them, and **the single most expensive setting a bulk
/// withdrawal of content runs into.** FTS5's own default, restored by
/// [`fts_begin_bulk_write`] and by [`fts_end_tombstone_burst`].
///
/// `0` disables delete-merging entirely — including from an explicit `'merge'`,
/// because `fts5IndexFindDeleteMerge` returns early on it — so it must never be
/// the resting value of an index. [`fts_begin_tombstone_burst`] is the only
/// thing that sets it, and always in a pair.
pub const FTS_DELETEMERGE: u8 = 10;
/// Set FTS5's delete-merge threshold. Best-effort; failure is logged.
fn fts_set_deletemerge(conn: &Connection, percent: u8) {
if let Err(e) = conn.execute(
"INSERT INTO searchabletext(searchabletext, rank) VALUES('deletemerge', ?1)",
[percent as i64],
) {
crate::log_warn!("FTS deletemerge failed (non-fatal): {}", e);
}
}
/// Stop FTS5 reclaiming tombstones *while* a pass is creating them, for a
/// caller that is about to delete a great many postings in one go and will call
/// [`fts_end_tombstone_burst`] when it is done.
///
/// # What it is worth
///
/// `examples/contentprobe.rs`, 40k rows of which 7,273 lose their content
/// (6,909 of them holding a posting), chunked deletes, committing per
/// `scope::SLICE`. Both arms, each pair from one run:
///
/// | arm | | fts | commit | pass | + merge | misses |
/// |---|---|---|---|---|---|---|
/// | plain | `deletemerge` 10 | 734 ms | 96 ms | **891 ms** | 892 ms | 97,130 |
/// | plain | `deletemerge` 0 | 86 ms | 11 ms | **141 ms** | 218 ms | 9,654 |
/// | keyed | `deletemerge` 10 | 1174 ms | 263 ms | **1576 ms** | 1579 ms | 96,181 |
/// | keyed | `deletemerge` 0 | 128 ms | 17 ms | **211 ms** | 373 ms | 9,476 |
///
/// 6.3x on the pass plain and 7.5x keyed; 4.1x and 4.2x once the trailing merge
/// is counted. Page-cache misses fall tenfold, which is why the keyed arm gains
/// more — every one of those pages was being decrypted and re-encrypted.
///
/// It also lands on a **smaller** index than the shipped path does — `%_data`
/// 1,753 rows against 1,886 — because one merge at the end consolidates better
/// than many mid-pass ones. That is the whole bargain: the mid-pass merges are
/// not just expensive, they are worse at the job.
///
/// End to end — `scope::advance` driven to completion the way the coordinator
/// drives it, on an idle machine — this and the two changes beside it are worth
/// **4.3x to 5.6x**, and the ratio *grows* with the index, because the levels
/// being needlessly rewritten grow with it:
///
/// | corpus | arm | before | after | |
/// |---|---|---|---|---|
/// | 40k | plain | 1,066 ms | **248 ms** | 4.3x |
/// | 40k | keyed | 1,814 ms | **417 ms** | 4.4x |
/// | 200k | plain | 4,600 ms | **925 ms** | 5.0x |
/// | 200k | keyed | 8,598 ms | **1,530 ms** | 5.6x |
///
/// The 40k pair is a true A/B: the same probe and corpus run against this tree
/// and against `HEAD` without these changes. Its *control* is what makes it a
/// measurement rather than two numbers from two binaries —
/// `contentprobe`'s `+clear(all)` stage reproduces the old shape in the probe's
/// own code, so it must not move between the builds, and it did not (1000 → 996
/// plain, 1801 → 1770 keyed). The 200k rows use that validated stage as the
/// "before", which is why they can come from a single run.
///
/// Measured and rejected alongside it: sorting each page's ids into rowid order
/// before deleting, which moved nothing (898 ms against 891 plain, 1565 against
/// 1576 keyed). FTS5 picks a tombstone page by *hashing* the rowid, so there is
/// no locality to restore.
///
/// # Why it is so large
///
/// Every contentless delete counts into the same write-counter that drives
/// `fts5IndexAutomerge`, and once a level passes this threshold
/// `fts5IndexFindDeleteMerge` picks it and rewrites the whole level — inline
/// with the scan, and then again as the pass keeps deleting. Rewriting the
/// full-text index is what building it was.
///
/// # The pairing is load-bearing
///
/// `deletemerge` is persisted in FTS5's `%_config` shadow table, so a value
/// left at `0` outlives the process and no later `'merge'` would ever reclaim a
/// tombstone again. [`fts_end_tombstone_burst`] restores it, and
/// [`fts_begin_bulk_write`] sets it unconditionally so that a crash between the
/// two is repaired by the next indexing run rather than being permanent.
pub fn fts_begin_tombstone_burst(conn: &Connection) {
fts_set_deletemerge(conn, 0);
}
/// Rounds of [`fts_finalize_after_text_indexing`] a burst may spend
/// consolidating before it gives up and leaves the rest to the next run.
///
/// Three is what the corpus in [`fts_begin_tombstone_burst`] needed; the cap is
/// above that so the common case finishes, and exists only so a pathological
/// index cannot hold the coordinator thread indefinitely.
const BURST_MERGE_ROUNDS: u32 = 8;
/// Restore delete-merging and consolidate what the burst left behind. The
/// mirror of [`fts_begin_tombstone_burst`]; see there for the measurements.
///
/// The order matters twice: the merge would reclaim nothing with the threshold
/// still at `0`, and the threshold must go back even when there is nothing to
/// merge, because a value left there would outlive the process.
///
/// # Why this merges to quiescence and `fts_finalize_after_text_indexing`
/// does not
///
/// A single 1000-page `'merge'` is the right bargain at the end of an indexing
/// run — whatever it leaves, the next run's merge finishes, and it was never
/// far behind. A burst is a different bargain: it deliberately built up a
/// backlog several times larger than a run ever does (`%_data` 4,931 rows
/// against the 1,886 the shipped path leaves), and searching against that until
/// some future run is a cost this pass created and should pay. It takes 77 ms
/// plain and 162 ms keyed, against the ~750 ms and ~1,450 ms the burst saved.
///
/// The quiescence signal is the `%_data` row count, and it has to be: measured
/// in `examples/pruneprobe.rs`, `sqlite3_changes()` after a `'merge'` reports
/// non-zero forever, so the obvious `while changes() != 0` never terminates.
pub fn fts_end_tombstone_burst(conn: &Connection) {
fts_set_deletemerge(conn, FTS_DELETEMERGE);
let mut last = fts_data_rows(conn);
for _ in 0..BURST_MERGE_ROUNDS {
fts_finalize_after_text_indexing(conn);
let now = fts_data_rows(conn);
if now == last {
return;
}
last = now;
}
}
/// Rows in FTS5's `%_data` shadow table — how much the full-text index is
/// physically holding, tombstones and all. `None` if it cannot be read, which
/// stops [`fts_end_tombstone_burst`]'s loop rather than spinning it.
fn fts_data_rows(conn: &Connection) -> Option<i64> {
conn.query_row("SELECT COUNT(*) FROM searchabletext_data", [], |r| r.get(0))
.ok()
}
/// Apply the write-side FTS5 settings, before a run starts writing. /// Apply the write-side FTS5 settings, before a run starts writing.
/// ///
/// `pgsz` is deliberately absent: it is not a per-run setting. Sweeping it /// `pgsz` is deliberately absent: it is not a per-run setting. Sweeping it
@ -123,8 +264,13 @@ const WRITE_CRISISMERGE: u8 = 32;
/// default 4050 stands there. Keyed is the opposite — SQLCipher's page reserve /// default 4050 stands there. Keyed is the opposite — SQLCipher's page reserve
/// makes 4050 a cliff — and that case is handled once at schema creation; see /// makes 4050 a cliff — and that case is handled once at schema creation; see
/// [`crate::db::schema::FTS_PGSZ_ENCRYPTED`]. /// [`crate::db::schema::FTS_PGSZ_ENCRYPTED`].
///
/// `deletemerge` is set even though this never lowers it: it is how an index
/// whose reconcile was killed mid-burst gets its tombstone reclamation back.
/// See [`fts_begin_tombstone_burst`].
pub fn fts_begin_bulk_write(conn: &Connection) { pub fn fts_begin_bulk_write(conn: &Connection) {
fts_set_automerge(conn, WRITE_AUTOMERGE); fts_set_automerge(conn, WRITE_AUTOMERGE);
fts_set_deletemerge(conn, FTS_DELETEMERGE);
if let Err(e) = conn.execute( if let Err(e) = conn.execute(
"INSERT INTO searchabletext(searchabletext, rank) VALUES('crisismerge', ?1)", "INSERT INTO searchabletext(searchabletext, rank) VALUES('crisismerge', ?1)",
[WRITE_CRISISMERGE as i64], [WRITE_CRISISMERGE as i64],

View file

@ -600,3 +600,38 @@ fn a_symlinked_root_yields_no_directories_when_following_is_off() {
std::fs::remove_dir_all(&base).ok(); std::fs::remove_dir_all(&base).ok();
} }
/// A reconcile killed mid-burst leaves `deletemerge` at 0, where no later
/// `'merge'` would reclaim a tombstone again. The next indexing run repairs it.
///
/// This is the whole reason [`fts_begin_bulk_write`] writes a value it never
/// lowers: the pairing in `scope::advance` covers the orderly cases, and this
/// covers the process simply not coming back.
#[test]
fn a_bulk_write_repairs_a_delete_merge_threshold_left_off() {
let dir = crate::testutil::scratch_dir("deletemerge-repair");
let db = dir.join("index.sqlite");
let conn = crate::db::open_or_recreate(db.to_str().unwrap(), "trigram").unwrap();
let threshold = || -> Option<i64> {
conn.query_row(
"SELECT v FROM searchabletext_config WHERE k = 'deletemerge'",
[],
|r| r.get(0),
)
.ok()
};
// What a killed burst leaves behind.
fts_begin_tombstone_burst(&conn);
assert_eq!(threshold(), Some(0), "the burst is in effect");
fts_begin_bulk_write(&conn);
assert_eq!(
threshold(),
Some(i64::from(FTS_DELETEMERGE)),
"the next run puts tombstone reclamation back"
);
std::fs::remove_dir_all(&dir).ok();
}

View file

@ -1,6 +1,34 @@
//! Brings a stored index back in line with a changed configuration without //! Brings a stored index back in line with a changed configuration without
//! rebuilding it. Nothing here stamps the stored configuration — the caller //! rebuilding it. Nothing here stamps the stored configuration — the caller
//! does, and only once the cursor reports finished; see [`outstanding_work`]. //! does, and only once the cursor reports finished; see [`outstanding_work`].
//!
//! # Where the time goes
//!
//! Almost entirely in FTS5: withdrawing content from a row tombstones its
//! posting, and that is 100x what deciding the row costs. `examples/contentprobe.rs`
//! and `examples/pruneprobe.rs` attribute a pass to a phase; the three
//! decisions that came out of them are [`SLICE`]-long transactions rather than
//! one per page, [`PagePlan`]'s chunked writes narrowed to the rows that can
//! actually hold what is being cleared, and
//! [`crate::file_handling::fts_begin_tombstone_burst`] — much the largest of
//! the three, and the place to read for why.
//!
//! Measured and **not** adopted, so they are not re-derived. Both looked
//! obvious; neither paid.
//!
//! - **A bigger page cache for the pass.** `PRAGMAS_INCREMENTAL`'s 4 MiB looks
//! far too small for a scan that rewrites across the whole index, and raising
//! it does exactly what it should to the miss count — 79,266 down to 6,499 at
//! 64 MiB — while the clock stays put (927 ms against 976). The misses were
//! never the expensive part; the tombstone writes behind them were.
//! - **Range-deleting a wholly-excluded directory** rather than paging through
//! it (`pruneprobe`'s `+subtree`). It does skip the page loop for every
//! doomed row, and costs 247 ms against `+fts(done)`'s 243.
//!
//! A third was never built, for the same reason: scanning by rowid instead of
//! by `(parent, name)` when `prune_scope` is off, which the root loop would
//! allow. Reading the pages is 2430 ms of a ~950 ms pass — there is nothing
//! there to win.
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::atomic::{AtomicBool, Ordering};
@ -11,7 +39,9 @@ use rusqlite::Connection;
use crate::config::{Config, IgnoreSet, IndexWork}; use crate::config::{Config, IgnoreSet, IndexWork};
use crate::db::repo; use crate::db::repo;
use crate::extract::Registry; use crate::extract::Registry;
use crate::file_handling::{content_extractable, fts_finalize_after_text_indexing, ExtractCursor}; use crate::file_handling::{
content_extractable, fts_begin_tombstone_burst, fts_end_tombstone_burst, ExtractCursor,
};
use crate::indexing::ReconcileProgress; use crate::indexing::ReconcileProgress;
/// How long [`advance`] may work before handing control back. This bounds /// How long [`advance`] may work before handing control back. This bounds
@ -180,6 +210,12 @@ impl WorkCursor {
self.finalized self.finalized
} }
/// Whether the row scan has reached the end of the last root. Distinct from
/// [`WorkCursor::done`], which also waits on the tombstone merge.
fn scan_done(&self) -> bool {
self.root_idx >= self.scope.roots.len()
}
pub fn progress(&self) -> ReconcileProgress { pub fn progress(&self) -> ReconcileProgress {
ReconcileProgress { ReconcileProgress {
examined: self.examined, examined: self.examined,
@ -227,6 +263,9 @@ pub fn advance(
let tx = conn let tx = conn
.transaction() .transaction()
.map_err(|e| format!("begin drop-root transaction: {}", e))?; .map_err(|e| format!("begin drop-root transaction: {}", e))?;
// A de-configured root can take every posting under it with it, which
// is the same burst the row scan makes; `advance` restores and merges.
fts_begin_tombstone_burst(&tx);
let removed = repo::delete_subtree(&tx, &range.lo, &range.hi)?; let removed = repo::delete_subtree(&tx, &range.lo, &range.hi)?;
tx.commit() tx.commit()
.map_err(|e| format!("commit drop-root transaction: {}", e))?; .map_err(|e| format!("commit drop-root transaction: {}", e))?;
@ -253,6 +292,7 @@ pub fn advance(
let tx = conn let tx = conn
.transaction() .transaction()
.map_err(|e| format!("begin drop-alias transaction: {}", e))?; .map_err(|e| format!("begin drop-alias transaction: {}", e))?;
fts_begin_tombstone_burst(&tx);
let removed = repo::delete_outside_ranges(&tx, &ranges)?; let removed = repo::delete_outside_ranges(&tx, &ranges)?;
tx.commit() tx.commit()
.map_err(|e| format!("commit drop-alias transaction: {}", e))?; .map_err(|e| format!("commit drop-alias transaction: {}", e))?;
@ -270,29 +310,91 @@ pub fn advance(
} }
cursor.total = Some(repo::row_count(conn)?); cursor.total = Some(repo::row_count(conn)?);
} }
let page = config.processing.batch_size.max(1) as i64; scan_rows(conn, config, registry, cursor, deadline, cancel)?;
let mut covered = CoverCache::default(); if !cursor.scan_done() {
while cursor.root_idx < cursor.scope.roots.len() { return Ok(());
if cancelled(cancel) { }
return Ok(()); }
}
// Deletions leave FTS tombstones; restoring the threshold and merging
// collapses them. Skipping the merge costs only tidiness — the next run's
// does the same — but the threshold must go back whether anything was
// deleted or not, since `scan_rows` lowers it before it knows.
if cancelled(cancel) {
return Ok(());
}
fts_end_tombstone_burst(conn);
cursor.finalized = true;
Ok(())
}
/// Rows one transaction may cover before it commits, whatever the clock says.
///
/// A backstop, not the primary limit: [`SLICE`] normally ends a transaction
/// first. It exists so that a fast disk and a wide slice cannot build an
/// unbounded set of dirty pages before anything is durable.
const COMMIT_ROWS: usize = 20_000;
/// Page every configured root, deciding and writing rows until the deadline.
///
/// One transaction spans as much of the slice as it can rather than one per
/// page. The read runs inside it, so each page sees the previous one's writes;
/// the cursor advances with the reads, so a rollback leaves it ahead of the
/// database — which is safe only because every caller of a failed [`advance`]
/// discards the cursor and nothing is stamped until one finishes. Cancellation
/// commits what it has: the work is idempotent, so a resumed cursor redoing it
/// would be correct too, but there is no reason to throw it away.
///
/// **The deadline is tested only after a page has been applied**, so a call
/// always makes progress. Testing it on entry instead is a spin: the caller
/// loops until the cursor finishes, and one that hands over an already-expired
/// deadline — which `advance`'s own `row_count` can produce on a large index,
/// and which `scope_tests` does deliberately — would never advance it.
fn scan_rows(
conn: &mut Connection,
config: &Config,
registry: &Registry,
cursor: &mut WorkCursor,
deadline: Instant,
cancel: &AtomicBool,
) -> Result<(), String> {
let page = config.processing.batch_size.max(1) as i64;
let mut covered = CoverCache::default();
let mut plan = PagePlan::default();
while !cursor.scan_done() {
if cancelled(cancel) {
return Ok(());
}
let tx = conn
.transaction()
.map_err(|e| format!("begin reconcile transaction: {}", e))?;
// Inside the transaction, so a rollback puts the threshold back with
// everything else; `advance` restores it for good when the pass ends.
// First thing in it, so the flush this implies has nothing to flush.
fts_begin_tombstone_burst(&tx);
let mut buffered = 0usize;
let mut spent = false;
while !cursor.scan_done() {
let root = &cursor.scope.roots[cursor.root_idx]; let root = &cursor.scope.roots[cursor.root_idx];
if cursor.after.0.is_empty() { if cursor.after.0.is_empty() {
// `(lo, "")` sorts below every row in the range — no stored name is empty. // `(lo, "")` sorts below every row in the range — no stored name is empty.
cursor.after = (root.lo.clone(), String::new()); cursor.after = (root.lo.clone(), String::new());
} }
let rows = let rows =
repo::rows_in_range_page(conn, &cursor.after.0, &cursor.after.1, &root.hi, page)?; repo::rows_in_range_page(&tx, &cursor.after.0, &cursor.after.1, &root.hi, page)?;
let Some(last) = rows.last() else { let Some(last) = rows.last() else {
// An exhausted root is not a page of work — moving to the next
// one must not be able to spend the slice on its own.
cursor.root_idx += 1; cursor.root_idx += 1;
cursor.after = (String::new(), String::new()); cursor.after = (String::new(), String::new());
continue; continue;
}; };
cursor.after = (last.parent.clone(), last.name.clone()); cursor.after = (last.parent.clone(), last.name.clone());
cursor.examined += rows.len(); cursor.examined += rows.len();
buffered += rows.len();
let root = cursor.scope.roots[cursor.root_idx].path.clone(); let root = cursor.scope.roots[cursor.root_idx].path.clone();
let (deleted, recontented) = apply_page( let (deleted, recontented) = apply_page(
conn, &tx,
config, config,
registry, registry,
&cursor.scope, &cursor.scope,
@ -300,24 +402,21 @@ pub fn advance(
&root, &root,
&rows, &rows,
&mut covered, &mut covered,
&mut plan,
)?; )?;
cursor.deleted += deleted; cursor.deleted += deleted;
cursor.recontented += recontented; cursor.recontented += recontented;
if Instant::now() >= deadline { if buffered >= COMMIT_ROWS || cancelled(cancel) || Instant::now() >= deadline {
return Ok(()); spent = true;
break;
} }
} }
tx.commit()
.map_err(|e| format!("commit reconcile transaction: {}", e))?;
if spent {
return Ok(());
}
} }
// Deletions leave FTS tombstones; a merge collapses them. Skipping it
// costs only tidiness — the next run's merge does the same.
if cancelled(cancel) {
return Ok(());
}
if cursor.deleted > 0 || cursor.recontented > 0 {
fts_finalize_after_text_indexing(conn);
}
cursor.finalized = true;
Ok(()) Ok(())
} }
@ -325,10 +424,85 @@ fn cancelled(cancel: &AtomicBool) -> bool {
cancel.load(Ordering::Relaxed) cancel.load(Ordering::Relaxed)
} }
/// Decide and write one page of rows. Returns `(deleted, recontented)`. /// One page's decisions, bucketed so every write goes out as a chunked
/// `IN (...)` list rather than a bound statement per row.
///
/// The buckets are by *stored* state as well as by destination, because that is
/// what says which of the three dependent writes a row actually needs: a
/// posting and a stored body exist only for `STATE_DONE`, a failure record only
/// for `STATE_FAILED` (`repo::leaving_done_always_takes_the_posting_with_it`
/// pins both). A `STATE_PENDING` row leaving for `STATE_NA` needs one `UPDATE`
/// and nothing else, where the per-row form spent four statements discovering
/// that twice over.
///
/// Reused across pages — `clear` keeps the capacity — so a whole pass allocates
/// these once.
#[derive(Default)]
struct PagePlan {
/// Rows leaving the index entirely, and the subset of them that can hold a
/// posting (see [`repo::delete_ids`]).
doomed: Vec<i64>,
doomed_content: Vec<i64>,
/// Rows keeping their posting but losing the snippet source.
stale_text: Vec<i64>,
/// Surviving rows changing `content_state`, and — across both — those whose
/// stored state says they have content or a failure record to clear first.
to_pending: Vec<i64>,
to_na: Vec<i64>,
restated_content: Vec<i64>,
restated_failure: Vec<i64>,
}
impl PagePlan {
fn clear(&mut self) {
for list in [
&mut self.doomed,
&mut self.doomed_content,
&mut self.stale_text,
&mut self.to_pending,
&mut self.to_na,
&mut self.restated_content,
&mut self.restated_failure,
] {
list.clear();
}
}
/// Note that a surviving row is changing state, and what it must shed first.
fn restate(&mut self, row: &repo::ScopeRow, to_pending: bool) {
if to_pending {
self.to_pending.push(row.id);
} else {
self.to_na.push(row.id);
}
match row.content_state {
repo::STATE_DONE => self.restated_content.push(row.id),
repo::STATE_FAILED => self.restated_failure.push(row.id),
_ => {}
}
}
/// Returns `(deleted, recontented)`.
fn write(&self, tx: &rusqlite::Transaction<'_>) -> Result<(usize, usize), String> {
let deleted = if self.doomed.is_empty() {
0
} else {
repo::delete_ids(tx, &self.doomed, &self.doomed_content)?
};
repo::drop_stored_text(tx, &self.stale_text)?;
repo::clear_content_for_ids(tx, &self.restated_content)?;
repo::clear_failed_for_ids(tx, &self.restated_failure)?;
repo::set_content_state(tx, &self.to_pending, repo::STATE_PENDING)?;
repo::set_content_state(tx, &self.to_na, repo::STATE_NA)?;
Ok((deleted, self.to_pending.len() + self.to_na.len()))
}
}
/// Decide one page of rows into `plan`, then write it. Returns
/// `(deleted, recontented)`.
#[allow(clippy::too_many_arguments)] #[allow(clippy::too_many_arguments)]
fn apply_page( fn apply_page(
conn: &mut Connection, tx: &rusqlite::Transaction<'_>,
config: &Config, config: &Config,
registry: &Registry, registry: &Registry,
scope: &Scope, scope: &Scope,
@ -336,20 +510,21 @@ fn apply_page(
root: &Path, root: &Path,
rows: &[repo::ScopeRow], rows: &[repo::ScopeRow],
covered: &mut CoverCache, covered: &mut CoverCache,
plan: &mut PagePlan,
) -> Result<(usize, usize), String> { ) -> Result<(usize, usize), String> {
let mut doomed: Vec<i64> = Vec::new(); plan.clear();
let mut stale_text: Vec<i64> = Vec::new();
let mut to_pending: Vec<i64> = Vec::new();
let mut to_na: Vec<i64> = Vec::new();
for row in rows { for row in rows {
let path = Path::new(&row.path); let path = Path::new(&row.path);
if work.prune_scope && !scope.covers_cached(root, path, covered) { if work.prune_scope && !scope.covers_cached(root, path, covered) {
doomed.push(row.id); plan.doomed.push(row.id);
if row.content_state == repo::STATE_DONE {
plan.doomed_content.push(row.id);
}
continue; continue;
} }
if work.drop_text { // Only a DONE row can have a `documents_text` body to drop.
stale_text.push(row.id); if work.drop_text && row.content_state == repo::STATE_DONE {
plan.stale_text.push(row.id);
} }
if work.reconcile_content || work.restore_text { if work.reconcile_content || work.restore_text {
// The walker's decision, recomputed. Both directions run whenever // The walker's decision, recomputed. Both directions run whenever
@ -357,37 +532,16 @@ fn apply_page(
let wants = row.size <= config.processing.maximum_text_file_size let wants = row.size <= config.processing.maximum_text_file_size
&& content_extractable(path, row.mime.as_deref(), config, registry); && content_extractable(path, row.mime.as_deref(), config, registry);
if !wants && row.content_state != repo::STATE_NA { if !wants && row.content_state != repo::STATE_NA {
to_na.push(row.id); plan.restate(row, false);
} else if wants } else if wants
&& (row.content_state == repo::STATE_NA && (row.content_state == repo::STATE_NA
|| (work.restore_text && row.content_state == repo::STATE_DONE)) || (work.restore_text && row.content_state == repo::STATE_DONE))
{ {
to_pending.push(row.id); plan.restate(row, true);
} }
} }
} }
plan.write(tx)
let tx = conn
.transaction()
.map_err(|e| format!("begin reconcile transaction: {}", e))?;
let deleted = if doomed.is_empty() {
0
} else {
repo::delete_ids(&tx, &doomed)?
};
if !stale_text.is_empty() {
repo::drop_stored_text(&tx, &stale_text)?;
}
for id in &to_pending {
repo::reset_content_pending(&tx, *id)?;
}
for id in &to_na {
repo::remove_content_for_id(&tx, *id)?;
repo::set_content_na(&tx, *id)?;
}
tx.commit()
.map_err(|e| format!("commit reconcile transaction: {}", e))?;
Ok((deleted, to_pending.len() + to_na.len()))
} }
/// The configuration the index was last built with, as far as /// The configuration the index was last built with, as far as

View file

@ -1,5 +1,6 @@
use super::*; use super::*;
use crate::walk::{walk_indexable_files, WalkEvent}; use crate::walk::{walk_indexable_files, WalkEvent};
use rusqlite::OptionalExtension;
use std::collections::HashSet; use std::collections::HashSet;
use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicBool;
use std::sync::Arc; use std::sync::Arc;
@ -211,6 +212,97 @@ fn the_scan_reports_its_way_through_every_row() {
std::fs::remove_dir_all(&db_dir).ok(); std::fs::remove_dir_all(&db_dir).ok();
} }
/// FTS5's delete-merge threshold is turned off for the length of a pass and
/// must come back on at the end of it.
///
/// Both halves are asserted, and both matter. Without the first the pass is
/// several times slower than it needs to be
/// (`file_handling::fts_begin_tombstone_burst` carries the table). Without the
/// second the index is left in a state where **no** later `'merge'` ever
/// reclaims a tombstone again — `fts5IndexFindDeleteMerge` returns early on a
/// zero threshold — and because FTS5 persists the setting in its `%_config`
/// shadow table, that outlives the process. It is a silent, permanent
/// degradation, which is exactly the kind of thing that needs a test rather
/// than a comment.
#[test]
fn a_pass_turns_delete_merging_off_and_puts_it_back() {
let root = tmp_tree("burst");
for i in 0..6 {
touch(&root.join(format!("f{}.log", i)));
}
touch(&root.join("keep.txt"));
let db_dir = tmp_tree("burst-db");
let db = empty_db(&db_dir);
let mut conn = crate::db::open_existing(db.to_str().unwrap(), true).unwrap();
let mut config = Config::default();
config.paths.indexing_paths = vec![root.to_string_lossy().into_owned()];
config.processing.batch_size = 1;
seed(&mut conn, &on_disk(&root));
// Read back out of FTS5's own `%_config`, not out of a value we remember:
// what outlives the process is what is written there.
let threshold = |conn: &Connection| -> Option<i64> {
conn.query_row(
"SELECT v FROM searchabletext_config WHERE k = 'deletemerge'",
[],
|r| r.get(0),
)
.optional()
.unwrap()
};
assert_eq!(
threshold(&conn),
None,
"a fresh index leaves FTS5 on its own default and records nothing"
);
let mut narrowed = config.clone();
narrowed.indexing.ignore_patterns = vec!["*.log".into()];
let work = crate::config::diff_actions(&config, &narrowed).work;
let mut cursor = WorkCursor::new(work, &narrowed).unwrap();
let registry = Registry::default_set();
let run = AtomicBool::new(false);
// One slice, with a deadline already past: the pass is under way and has
// committed at least one page, so the threshold is off and durably so.
advance(
&mut conn,
&narrowed,
&registry,
&mut cursor,
Instant::now(),
&run,
)
.unwrap();
assert!(!cursor.done(), "one page cannot have finished seven rows");
assert_eq!(
threshold(&conn),
Some(0),
"delete-merging is off while the pass is creating tombstones"
);
while !cursor.done() {
advance(
&mut conn,
&narrowed,
&registry,
&mut cursor,
Instant::now(),
&run,
)
.unwrap();
}
assert_eq!(
threshold(&conn),
Some(i64::from(crate::file_handling::FTS_DELETEMERGE)),
"a finished pass leaves tombstone reclamation working again"
);
std::fs::remove_dir_all(&root).ok();
std::fs::remove_dir_all(&db_dir).ok();
}
/// The cursor is left un-finished, so nothing downstream records the config /// The cursor is left un-finished, so nothing downstream records the config
/// as reconciled. Rows already reached stay gone: the pass is idempotent. /// as reconciled. Rows already reached stay gone: the pass is idempotent.
#[test] #[test]

View file

@ -286,8 +286,35 @@ pub struct SeedSpec {
/// file cannot be reopened without it. Ignored on a plain arm, which has /// file cannot be reopened without it. Ignored on a plain arm, which has
/// no reserve. /// no reserve.
pub hmac: Option<crate::db::schema::HmacMode>, pub hmac: Option<crate::db::schema::HmacMode>,
/// `(extension, mime)` pairs cycled across the rows, deciding what a
/// `content_extensions` filter can select. [`EXT_PLAIN`] — one pair, so
/// every row is `.txt`/`text/plain` — is what every harness measuring
/// search or indexing wants, and it is the default so their corpora are
/// byte-identical to what they have always been.
///
/// It exists for `contentprobe`, where a filter that either takes the
/// whole index or none of it answers nothing. **A mix whose length shares
/// a factor with `content_every` puts every document behind the same few
/// extensions** — the degenerate-corpus trap `pruneprobe` documents
/// against its own strides — so a harness using this should assert the
/// fractions it ends up with rather than trusting the arithmetic.
pub ext_mix: &'static [(&'static str, &'static str)],
/// One row in every `pending_every` that *would* hold content is left in
/// the pending queue instead — born `STATE_PENDING` and never extracted,
/// which is the residue an interrupted content pass leaves behind. `0`
/// (the default) seeds none.
///
/// It exists because such a row is the case a re-decision can skip the
/// most work on: it is neither `STATE_NA` (so a narrowed filter must still
/// flip it) nor `STATE_DONE` (so it has no posting and no stored text to
/// clear). A corpus without any cannot tell whether clearing content for
/// rows that cannot hold it costs anything.
pub pending_every: usize,
} }
/// The single-extension corpus every harness but `contentprobe` seeds.
pub const EXT_PLAIN: &[(&str, &str)] = &[("txt", "text/plain")];
impl Default for SeedSpec { impl Default for SeedSpec {
fn default() -> SeedSpec { fn default() -> SeedSpec {
SeedSpec { SeedSpec {
@ -306,6 +333,8 @@ impl Default for SeedSpec {
page_size: None, page_size: None,
pgsz: None, pgsz: None,
hmac: None, hmac: None,
ext_mix: EXT_PLAIN,
pending_every: 0,
} }
} }
} }
@ -314,7 +343,7 @@ impl Default for SeedSpec {
/// measurement harnesses so they all describe the same corpus. /// measurement harnesses so they all describe the same corpus.
pub fn seed_index(path: &std::path::Path, spec: &SeedSpec) { pub fn seed_index(path: &std::path::Path, spec: &SeedSpec) {
use crate::db::repo::{insert_file, set_content_done, NewFile}; use crate::db::repo::{insert_file, set_content_done, NewFile};
use crate::mime::FileType; use crate::mime::mime_to_type;
// Before the open, not after: the profile decides how the file is // Before the open, not after: the profile decides how the file is
// *created*, and on a keyed file it decides whether it can be read at all. // *created*, and on a keyed file it decides whether it can be read at all.
@ -335,6 +364,19 @@ pub fn seed_index(path: &std::path::Path, spec: &SeedSpec) {
.unwrap(); .unwrap();
} }
let mut rng = Lcg::new(0x5eed); let mut rng = Lcg::new(0x5eed);
let ext_mix = if spec.ext_mix.is_empty() {
EXT_PLAIN
} else {
spec.ext_mix
};
// A row gets content only if an extractor would have claimed its MIME, so
// the seeded `content_state` is what a real run under an *unfiltered*
// config would have left. Without this a corpus of mixed extensions is
// born disagreeing with its own configuration, and the first reconcile
// against it spends its time repairing the seed rather than applying the
// edit. `EXT_PLAIN` is claimed by the plaintext extractor, so the
// single-extension corpus every other harness seeds is unchanged.
let registry = crate::extract::Registry::default_set();
// Spacing, not a random draw: a cluster at the front would let a pass // Spacing, not a random draw: a cluster at the front would let a pass
// stop early and report a fraction of the work a real rare query costs. // stop early and report a fraction of the work a real rare query costs.
let name_stride = spec.files / spec.needle_names.max(1); let name_stride = spec.files / spec.needle_names.max(1);
@ -346,10 +388,14 @@ pub fn seed_index(path: &std::path::Path, spec: &SeedSpec) {
for i in 0..spec.files { for i in 0..spec.files {
let w1 = rng.pick(WORDS); let w1 = rng.pick(WORDS);
let w2 = rng.pick(WORDS); let w2 = rng.pick(WORDS);
// Extension and MIME move together: a row whose name says `.pdf` and
// whose MIME says `text/plain` would let `content_extractable`'s two
// halves disagree, which is exactly what a content filter is testing.
let (ext, mime) = ext_mix[i % ext_mix.len()];
let name = if spec.needle_names > 0 && i % name_stride.max(1) == 0 { let name = if spec.needle_names > 0 && i % name_stride.max(1) == 0 {
format!("{}-{}-{:07}.txt", w1, NEEDLE, i) format!("{}-{}-{:07}.{}", w1, NEEDLE, i, ext)
} else { } else {
format!("{}-{}-{:07}.txt", w1, w2, i) format!("{}-{}-{:07}.{}", w1, w2, i, ext)
}; };
// Stored parents always end in a separator; see `dir_to_db_parent`. // Stored parents always end in a separator; see `dir_to_db_parent`.
// Deeper segments are derived from the directory index, not the file // Deeper segments are derived from the directory index, not the file
@ -376,6 +422,11 @@ pub fn seed_index(path: &std::path::Path, spec: &SeedSpec) {
} }
bytes bytes
}); });
// `needs_content` is what the row is *born* as — `insert_file` gives it
// `STATE_PENDING`. Skipping the `set_content_done` below is therefore
// all it takes to leave one behind in the queue.
let needs_content = i % spec.content_every.max(1) == 0 && registry.supports(mime);
let extracted = needs_content && (spec.pending_every == 0 || i % spec.pending_every != 0);
let id = insert_file( let id = insert_file(
&tx, &tx,
&NewFile { &NewFile {
@ -383,15 +434,15 @@ pub fn seed_index(path: &std::path::Path, spec: &SeedSpec) {
parent: &dir, parent: &dir,
size: 4096, size: 4096,
mtime: 1_700_000_000 + i as u64, mtime: 1_700_000_000 + i as u64,
mime: Some("text/plain"), mime: Some(mime),
ftype: FileType::TEXT, ftype: mime_to_type(mime),
hash: hash.as_ref().map(|h| h.as_slice()), hash: hash.as_ref().map(|h| h.as_slice()),
needs_content: i % spec.content_every.max(1) == 0, needs_content,
}, },
) )
.unwrap() .unwrap()
.expect("unique path"); .expect("unique path");
if i % spec.content_every.max(1) == 0 { if extracted {
let mut body: Vec<&str> = (0..spec.body_words).map(|_| *rng.pick(WORDS)).collect(); let mut body: Vec<&str> = (0..spec.body_words).map(|_| *rng.pick(WORDS)).collect();
if spec.needle_docs > 0 && i % doc_stride.max(1) == 0 { if spec.needle_docs > 0 && i % doc_stride.max(1) == 0 {
// Mid-body, so a snippet window has to be cut around it. // Mid-body, so a snippet window has to be cut around it.

View file

@ -109,4 +109,8 @@ windows-sys = { version = "0.59", features = [
"Win32_Storage_FileSystem", "Win32_Storage_FileSystem",
"Win32_System_IO", "Win32_System_IO",
"Win32_Security", "Win32_Security",
# The foreground handshake (`activate`) and native raise (`activate::raise`):
# GetCurrentProcessId, AllowSetForegroundWindow / SetForegroundWindow.
"Win32_System_Threading",
"Win32_UI_WindowsAndMessaging",
] } ] }

View file

@ -7,9 +7,12 @@
//! box whether or not the app was started", because a shortcut an application //! box whether or not the app was started", because a shortcut an application
//! registers for itself cannot fire while the application is not there. //! registers for itself cannot fire while the application is not there.
//! //!
//! The message carries nothing: "come forward" is the whole protocol, and //! The message carries nothing: "come forward" is the whole protocol. On
//! the reply exists only so the sender can tell a live instance from a //! unix the reply exists only so the sender can tell a live instance from a
//! leftover socket. An xdg-activation token would be the natural thing to //! leftover socket; on Windows it instead carries the server's PID, which
//! the sender feeds to `AllowSetForegroundWindow` so the running window may
//! actually take the foreground — see the `#[cfg(windows)]` module below.
//! An xdg-activation token would be the natural thing to
//! carry — it is what a compositor wants before letting a background client //! carry — it is what a compositor wants before letting a background client
//! take focus — but nothing downstream can consume one: winit 0.30 applies a //! take focus — but nothing downstream can consume one: winit 0.30 applies a
//! token only in `WindowAttributes`, and egui's `ViewportBuilder` has no //! token only in `WindowAttributes`, and egui's `ViewportBuilder` has no
@ -28,6 +31,21 @@ use std::sync::atomic::{AtomicBool, Ordering};
/// the same thing as one. /// the same thing as one.
static PENDING: AtomicBool = AtomicBool::new(false); static PENDING: AtomicBool = AtomicBool::new(false);
/// The most a PID reply can be: a 32-bit PID is at most ten digits, and the
/// newline ends it. Also the pipe's buffer size, so the server's write never
/// blocks on a client that reads nothing.
#[cfg_attr(not(windows), allow(dead_code))]
const PID_REPLY_CAP: usize = 16;
/// The server's PID out of its reply: ASCII decimal up to a newline.
/// Anything else — truncation, garbage, an empty read — is `None`, which
/// skips the foreground grant rather than failing the activation.
#[cfg_attr(not(windows), allow(dead_code))]
fn parse_pid_reply(reply: &[u8]) -> Option<u32> {
let line = reply.split(|&b| b == b'\n').next()?;
std::str::from_utf8(line).ok()?.parse().ok()
}
/// The socket identifying the instance that `config_path` configures. /// The socket identifying the instance that `config_path` configures.
/// ///
/// **Keyed by the config file, not the index.** The index path is a setting /// **Keyed by the config file, not the index.** The index path is a setting
@ -190,9 +208,15 @@ mod imp {
/// Runs until the process exits; a connection is one activation. /// Runs until the process exits; a connection is one activation.
fn serve(ctx: &egui::Context, listener: UnixListener) { fn serve(ctx: &egui::Context, listener: UnixListener) {
for stream in listener.incoming() { for stream in listener.incoming() {
let Ok(stream) = stream else { continue }; let Ok(mut stream) = stream else { continue };
match answer(stream) { match request(&mut stream) {
Ok(()) => fire(ctx), // Fire *before* the ack: a client that saw the reply may act
// on "delivered", and delivered means the window was already
// asked to come forward.
Ok(()) => {
fire(ctx);
let _ = acknowledge(&mut stream);
}
// One stalled or truncated peer must not stop the loop, and // One stalled or truncated peer must not stop the loop, and
// must not raise the window on a request it never finished. // must not raise the window on a request it never finished.
Err(e) => quicksearch_core::log_warn!("a search shortcut request: {}", e), Err(e) => quicksearch_core::log_warn!("a search shortcut request: {}", e),
@ -200,13 +224,13 @@ mod imp {
} }
} }
/// Read the request and acknowledge it. /// Read the request.
/// ///
/// Hostile input is the norm rather than the exception: any process of /// Hostile input is the norm rather than the exception: any process of
/// this user can connect. The read is bounded in both bytes and time, so /// this user can connect. The read is bounded in both bytes and time, so
/// a peer that connects and stalls cannot wedge the one thread that /// a peer that connects and stalls cannot wedge the one thread that
/// answers every activation. /// answers every activation.
pub(super) fn answer(mut stream: UnixStream) -> std::io::Result<()> { pub(super) fn request(stream: &mut UnixStream) -> std::io::Result<()> {
let timeout = std::time::Duration::from_secs(5); let timeout = std::time::Duration::from_secs(5);
stream.set_read_timeout(Some(timeout))?; stream.set_read_timeout(Some(timeout))?;
stream.set_write_timeout(Some(timeout))?; stream.set_write_timeout(Some(timeout))?;
@ -214,8 +238,12 @@ mod imp {
// One byte is the whole request; the cap is what keeps a peer from // One byte is the whole request; the cap is what keeps a peer from
// holding this thread for as long as it cares to send. // holding this thread for as long as it cares to send.
let mut scratch = [0u8; 1]; let mut scratch = [0u8; 1];
stream.read_exact(&mut scratch)?; stream.read_exact(&mut scratch)
}
/// The reply that lets the sender tell a live instance from a leftover
/// socket file.
pub(super) fn acknowledge(stream: &mut UnixStream) -> std::io::Result<()> {
stream.write_all(b"\n")?; stream.write_all(b"\n")?;
stream.flush() stream.flush()
} }
@ -224,11 +252,17 @@ mod imp {
/// The same handshake over a named pipe, which is what Windows has instead /// The same handshake over a named pipe, which is what Windows has instead
/// of a unix socket. /// of a unix socket.
/// ///
/// **One deliberate difference: there is no reply.** A named pipe exists only /// **One deliberate difference: the reply is the foreground grant, not a
/// while a server holds an instance open — there is no file left behind — so /// liveness check.** A named pipe exists only while a server holds an
/// a successful `CreateFileW` already proves a live instance accepted us, and /// instance open — there is no file left behind — so a successful
/// the reply the unix side needs to tell a listener from a leftover socket /// `CreateFileW` already proves a live instance accepted us. What Windows
/// would be dead weight here. /// *does* need is permission: `SetForegroundWindow` is refused to a process
/// the user did not just interact with, and the running instance is exactly
/// that. The `--toggle` process was launched by the user's keypress and so
/// holds the right — and may donate it with `AllowSetForegroundWindow`, given
/// the server's PID. The server therefore opens every connection by writing
/// its PID, and the sender grants before sending the request, so the grant
/// is always in force by the time the server raises.
#[cfg(windows)] #[cfg(windows)]
mod imp { mod imp {
use super::*; use super::*;
@ -247,6 +281,8 @@ mod imp {
ConnectNamedPipe, CreateNamedPipeW, DisconnectNamedPipe, PIPE_READMODE_BYTE, ConnectNamedPipe, CreateNamedPipeW, DisconnectNamedPipe, PIPE_READMODE_BYTE,
PIPE_TYPE_BYTE, PIPE_UNLIMITED_INSTANCES, PIPE_WAIT, PIPE_TYPE_BYTE, PIPE_UNLIMITED_INSTANCES, PIPE_WAIT,
}; };
use windows_sys::Win32::System::Threading::GetCurrentProcessId;
use windows_sys::Win32::UI::WindowsAndMessaging::AllowSetForegroundWindow;
/// `\\.\pipe\quicksearch-<key>`, keyed exactly as the unix socket is, so /// `\\.\pipe\quicksearch-<key>`, keyed exactly as the unix socket is, so
/// the two processes agree by the same rule on both platforms. /// the two processes agree by the same rule on both platforms.
@ -300,6 +336,29 @@ mod imp {
return false; return false;
} }
let pipe = Handle(handle); let pipe = Handle(handle);
// The server opens with its PID; hand it the foreground right we
// hold from the user's keypress *before* asking it to raise. A
// reply that does not parse skips the grant — the raise then
// degrades to a taskbar flash rather than the request being lost.
let mut reply = [0u8; PID_REPLY_CAP];
let mut got = 0u32;
// SAFETY: `reply` and `got` are live for the call; the buffer
// length passed is the buffer's real length.
let ok = unsafe {
ReadFile(
pipe.0,
reply.as_mut_ptr(),
reply.len() as u32,
&mut got,
std::ptr::null_mut(),
)
};
if ok != 0 {
if let Some(pid) = parse_pid_reply(&reply[..got as usize]) {
// SAFETY: no pointers; any PID value is acceptable input.
unsafe { AllowSetForegroundWindow(pid) };
}
}
let mut written = 0u32; let mut written = 0u32;
// SAFETY: a one-byte buffer and an output slot, both live here. // SAFETY: a one-byte buffer and an output slot, both live here.
let ok = unsafe { let ok = unsafe {
@ -349,8 +408,8 @@ mod imp {
PIPE_ACCESS_DUPLEX, PIPE_ACCESS_DUPLEX,
PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT,
PIPE_UNLIMITED_INSTANCES, PIPE_UNLIMITED_INSTANCES,
16, PID_REPLY_CAP as u32,
16, PID_REPLY_CAP as u32,
0, 0,
std::ptr::null(), std::ptr::null(),
) )
@ -368,6 +427,28 @@ mod imp {
// the read below is what decides, so it is not checked here. // the read below is what decides, so it is not checked here.
let _ = connected; let _ = connected;
// Our PID first, before reading anything: the client turns it
// into a foreground grant and only then sends the request, so
// the grant precedes the raise however the two threads interleave.
// SAFETY: reads this process's own id, always valid.
let pid = format!("{}\n", unsafe { GetCurrentProcessId() });
let mut written = 0u32;
// SAFETY: the buffer and output slot are live; the length passed
// is the buffer's real length.
let wrote = unsafe {
WriteFile(
pipe.0,
pid.as_ptr(),
pid.len() as u32,
&mut written,
std::ptr::null_mut(),
)
};
if wrote != 0 {
// SAFETY: the handle is open for the length of this call.
unsafe { FlushFileBuffers(pipe.0) };
}
let mut scratch = [0u8; 1]; let mut scratch = [0u8; 1];
let mut read = 0u32; let mut read = 0u32;
// SAFETY: a one-byte buffer and an output slot, both live here. // SAFETY: a one-byte buffer and an output slot, both live here.
@ -380,12 +461,14 @@ mod imp {
std::ptr::null_mut(), std::ptr::null_mut(),
) )
}; };
// SAFETY: the handle is open for the length of this call.
unsafe { DisconnectNamedPipe(pipe.0) };
// A peer that connected and said nothing is not an activation. // A peer that connected and said nothing is not an activation.
// Fired before the disconnect, so a client watching the pipe
// close can already rely on the window having been asked.
if ok != 0 && read == 1 { if ok != 0 && read == 1 {
fire(ctx); fire(ctx);
} }
// SAFETY: the handle is open for the length of this call.
unsafe { DisconnectNamedPipe(pipe.0) };
} }
} }
} }
@ -436,6 +519,25 @@ mod tests {
assert_ne!(a, b); assert_ne!(a, b);
} }
/// The Windows reply parser, which faces whatever a squatting process
/// cares to write into the well-known pipe name.
#[test]
fn the_pid_reply_parses_strictly() {
assert_eq!(parse_pid_reply(b"12345\n"), Some(12345));
assert_eq!(parse_pid_reply(b"1\nrest ignored"), Some(1));
for garbage in [
&b""[..],
b"\n",
b"-4\n",
b"12345678901234567890\n", // overflows a u32
b"abc\n",
b"12 34\n",
b"\xff\xfe\n",
] {
assert_eq!(parse_pid_reply(garbage), None, "{:?}", garbage);
}
}
#[test] #[test]
fn a_pending_activation_is_consumed_once() { fn a_pending_activation_is_consumed_once() {
let _serial = pending_guard(); let _serial = pending_guard();
@ -482,12 +584,13 @@ mod tests {
let sender = std::thread::spawn(move || signal(&db)); let sender = std::thread::spawn(move || signal(&db));
let stream = listener let mut stream = listener
.incoming() .incoming()
.next() .next()
.expect("a connection") .expect("a connection")
.expect("accepted"); .expect("accepted");
imp::answer(stream).expect("a well-formed request"); imp::request(&mut stream).expect("a well-formed request");
imp::acknowledge(&mut stream).expect("acknowledged");
assert!(sender.join().expect("sender"), "the client saw the reply"); assert!(sender.join().expect("sender"), "the client saw the reply");
} }
@ -525,12 +628,15 @@ mod tests {
let peer = UnixStream::connect(&path).expect("connect"); let peer = UnixStream::connect(&path).expect("connect");
drop(peer); drop(peer);
let stream = listener let mut stream = listener
.incoming() .incoming()
.next() .next()
.expect("a connection") .expect("a connection")
.expect("accepted"); .expect("accepted");
assert!(imp::answer(stream).is_err(), "an empty request is refused"); assert!(
imp::request(&mut stream).is_err(),
"an empty request is refused"
);
} }
/// A config path unique to this test. Never opened as a file — only /// A config path unique to this test. Never opened as a file — only
@ -585,6 +691,16 @@ mod tests {
std::thread::sleep(std::time::Duration::from_millis(20)); std::thread::sleep(std::time::Duration::from_millis(20));
} }
assert!(delivered, "the listener never answered"); assert!(delivered, "the listener never answered");
assert!(take_pending(), "and the window was asked to come forward"); // Unlike unix there is no ack after the fire: the server reads the
// request after the client's write returns, so give it a moment.
let mut fired = false;
for _ in 0..100 {
if take_pending() {
fired = true;
break;
}
std::thread::sleep(std::time::Duration::from_millis(10));
}
assert!(fired, "the window was never asked to come forward");
} }
} }

View file

@ -20,8 +20,14 @@
//! which shows up as a highlighted task entry. Closing that gap means //! which shows up as a highlighted task entry. Closing that gap means
//! patching both winit (to activate a live surface) and eframe (to carry //! patching both winit (to activate a live surface) and eframe (to carry
//! the token), the way `vendor/` already patches two other crates. //! the token), the way `vendor/` already patches two other crates.
//! * **Windows**: `SetForegroundWindow` is refused to background processes, //! * **Windows**: `SetForegroundWindow` is refused to background processes —
//! so the same limit applies to a process that did not just receive input. //! but the `--toggle` sender was launched by the user's keypress and
//! donates its right over the pipe with `AllowSetForegroundWindow` (see
//! `crate::activate`'s Windows module), after which [`win32_activate`]'s
//! `SetForegroundWindow` is honoured. The in-app hotkey path needs no
//! grant: the press was delivered to this process. Only the portal-less
//! case with no grant — e.g. some other process poking the pipe — degrades
//! to a taskbar flash.
/// Whether this is a Wayland session, where an already-open window cannot be /// Whether this is a Wayland session, where an already-open window cannot be
/// raised. The Settings tab says so rather than letting the shortcut look /// raised. The Settings tab says so rather than letting the shortcut look
@ -50,7 +56,13 @@ pub fn raise(ctx: &egui::Context, frame: &eframe::Frame) {
return; return;
} }
} }
#[cfg(not(all(unix, not(target_os = "macos"))))] #[cfg(windows)]
{
if win32_activate(frame) {
return;
}
}
#[cfg(not(any(all(unix, not(target_os = "macos")), windows)))]
{ {
let _ = frame; let _ = frame;
} }
@ -60,6 +72,56 @@ pub fn raise(ctx: &egui::Context, frame: &eframe::Frame) {
ctx.send_viewport_cmd(egui::ViewportCommand::Focus); ctx.send_viewport_cmd(egui::ViewportCommand::Focus);
} }
/// Restore and foreground our window with the Win32 calls themselves.
/// `false` falls back to winit's viewport commands.
///
/// Not `ViewportCommand::Focus`: winit's `focus_window` routes through the
/// same `SetForegroundWindow`, but only after the event loop wakes and with
/// its own preconditions, and it neither restores a minimised window nor
/// reports failure. Calling the API here keeps restore-then-foreground in
/// one place, immediately, while the `AllowSetForegroundWindow` grant from
/// the `--toggle` sender is fresh.
#[cfg(windows)]
fn win32_activate(frame: &eframe::Frame) -> bool {
use raw_window_handle::{HasWindowHandle, RawWindowHandle};
use windows_sys::Win32::UI::WindowsAndMessaging::{
IsIconic, SetForegroundWindow, ShowWindow, SW_RESTORE,
};
let handle = match frame.window_handle() {
Ok(handle) => handle,
Err(e) => {
quicksearch_core::log_warn!("raising the window: no window handle: {}", e);
return false;
}
};
let hwnd = match handle.as_raw() {
RawWindowHandle::Win32(win32) => win32.hwnd.get() as _,
other => {
quicksearch_core::log_warn!("raising the window: not a Win32 window: {:?}", other);
return false;
}
};
// SAFETY: `hwnd` is this process's live window for the whole call; these
// APIs accept any window handle and merely fail on a bad one.
unsafe {
if IsIconic(hwnd) != 0 {
ShowWindow(hwnd, SW_RESTORE);
}
if SetForegroundWindow(hwnd) == 0 {
// No grant in force (see the module docs): the most Windows
// allows from here is a taskbar flash, which the winit fallback
// produces. Logged so a shortcut that only flashes is traceable.
quicksearch_core::log_warn!(
"raising the window: SetForegroundWindow was refused; \
flashing the taskbar instead"
);
return false;
}
}
true
}
// The X connection used for activation, kept open across presses. // The X connection used for activation, kept open across presses.
// Thread-local because `raise` only ever runs on the UI thread, and held // Thread-local because `raise` only ever runs on the UI thread, and held
// rather than reconnected because a connect per keypress is both wasteful // rather than reconnected because a connect per keypress is both wasteful

View file

@ -391,6 +391,8 @@ impl QuickSearchApp {
// Only when moved: on Wayland re-registering opens a new portal // Only when moved: on Wayland re-registering opens a new portal
// session, which some desktops confirm with the user. // session, which some desktops confirm with the user.
crate::hotkey::apply(&new.ui.search_hotkey); crate::hotkey::apply(&new.ui.search_hotkey);
// And the system-wide binding follows, where one is written.
crate::shortcut_setup::hotkey_changed(&new.ui.search_hotkey);
} }
if new.ui.color_scheme != self.cfg.ui.color_scheme { if new.ui.color_scheme != self.cfg.ui.color_scheme {
apply_theme(ctx, &new.ui.color_scheme); apply_theme(ctx, &new.ui.color_scheme);

View file

@ -213,20 +213,20 @@ impl QuickSearchApp {
if actions.focus_search { if actions.focus_search {
self.search.request_focus(); self.search.request_focus();
} }
// Live, like the Settings slider on Apply — but saved only when the // Only ever emitted by the page's Apply button, so applying and
// drag ends, so crossing the slider does not rewrite the config file // saving belong together — the same pair the Settings tab's Apply
// on every frame. // performs.
if let Some(scale) = actions.set_scale { if let Some(scale) = actions.set_scale {
self.cfg.ui.scale = scale; self.cfg.ui.scale = scale;
ctx.set_zoom_factor(super::clamp_scale(scale)); ctx.set_zoom_factor(super::clamp_scale(scale));
if actions.save_scale { self.save_cfg();
self.save_cfg();
}
} }
// Registered as it is captured, not on an Apply the tour has no // Registered as it is captured, not on an Apply the tour has no
// button for — the page says it takes effect at once. // button for — the page says it takes effect at once.
if let Some(hotkey) = actions.set_hotkey { if let Some(hotkey) = actions.set_hotkey {
crate::hotkey::apply(&hotkey); crate::hotkey::apply(&hotkey);
// The system-wide binding follows, where one is written.
crate::shortcut_setup::hotkey_changed(&hotkey);
self.cfg.ui.search_hotkey = hotkey; self.cfg.ui.search_hotkey = hotkey;
self.save_cfg(); self.save_cfg();
} }

View file

@ -232,34 +232,50 @@ fn ranking_section(ui: &mut egui::Ui) {
.spacing([CELL_SPACING, 5.0]) .spacing([CELL_SPACING, 5.0])
.striped(true) .striped(true)
.show(ui, |ui| { .show(ui, |ui| {
let row = |ui: &mut egui::Ui, tier: &str, what: &str| { // Each tier's chip wears the colour its results wear in the Rank
ui.strong(tier); // column, keyed by the *first* cascade stage the collapsed tier
// covers (see the rank table in `search::cascade`): exact name
// 12, name contains 34, text inside 56, fuzzy 78, path 911.
let row = |ui: &mut egui::Ui, stage: u8, tier: &str, what: &str| {
ui.label(
egui::RichText::new(format!(" {} ", tier))
.strong()
.background_color(crate::color::rank_tier_color(stage))
// The same near-black the Search tab's chips carry,
// which every ramp colour holds contrast against.
.color(egui::Color32::from_rgb(32, 32, 32)),
);
cell(ui, prose, what); cell(ui, prose, what);
ui.end_row(); ui.end_row();
}; };
row( row(
ui, ui,
1,
"Exact name", "Exact name",
"the file is called exactly what you typed", "the file is called exactly what you typed",
); );
row( row(
ui, ui,
3,
"Name contains", "Name contains",
"what you typed appears somewhere in the file's name", "what you typed appears somewhere in the file's name",
); );
row( row(
ui, ui,
5,
"Text inside", "Text inside",
"the words are in the file's contents, most mentions first", "the words are in the file's contents, most mentions first",
); );
row( row(
ui, ui,
7,
"Close spelling", "Close spelling",
"a name or some text within a typo or two of what you typed, \ "a name or some text within a typo or two of what you typed, \
only while Fuzzy is ticked", only while Fuzzy is ticked",
); );
row( row(
ui, ui,
9,
"Path only", "Path only",
"nothing in the name or the text matched, but a folder along \ "nothing in the name or the text matched, but a folder along \
the way did", the way did",
@ -601,8 +617,9 @@ mod tests {
#[test] #[test]
fn a_window_narrower_than_the_column_reflows_rather_than_clipping() { fn a_window_narrower_than_the_column_reflows_rather_than_clipping() {
// 640 is the smallest window the app allows, and the UI scale // 640 is the smallest window the app allows, and the UI scale
// divides it: 400 is roughly that window at 1.6x. // divides it: 400 is roughly that window at 1.6x, and 250 is it at
for width in [400.0_f32, 480.0, 560.0, 620.0] { // the 2.5x ceiling — the narrowest layout the app can produce.
for width in [250.0_f32, 320.0, 400.0, 480.0, 560.0, 620.0] {
let ctx = crate::test_ui::ctx(); let ctx = crate::test_ui::ctx();
let input = crate::test_ui::raw_input(egui::vec2(width, 6000.0), vec![]); let input = crate::test_ui::raw_input(egui::vec2(width, 6000.0), vec![]);
let out = ctx.run(input, |ctx| { let out = ctx.run(input, |ctx| {
@ -624,6 +641,39 @@ mod tests {
} }
} }
/// Every tier chip wears the Rank column's own colour for its first
/// cascade stage — the chips exist to demonstrate the blue→red ramp the
/// paragraph under the table describes.
#[test]
fn the_ranking_tiers_wear_the_rank_colors() {
let ctx = crate::test_ui::ctx();
let input = crate::test_ui::raw_input(egui::vec2(1000.0, 4000.0), vec![]);
let out = ctx.run(input, |ctx| {
egui::CentralPanel::default().show(ctx, |ui| {
super::ui(ui);
});
});
// A RichText background is a section format in the galley, not a
// separate rect shape.
let mut backgrounds = Vec::new();
for clipped in &out.shapes {
if let egui::epaint::Shape::Text(text) = &clipped.shape {
for section in &text.galley.job.sections {
backgrounds.push(section.format.background);
}
}
}
for stage in [1u8, 3, 5, 7, 9] {
let color = crate::color::rank_tier_color(stage);
assert!(
backgrounds.contains(&color),
"no chip painted in stage {}'s colour {:?}",
stage,
color
);
}
}
/// The near-miss column is half of what the examples teach, so the /// The near-miss column is half of what the examples teach, so the
/// narrow layout has to keep it rather than dropping to pattern-only. /// narrow layout has to keep it rather than dropping to pattern-only.
#[test] #[test]

View file

@ -156,6 +156,24 @@ impl Binding {
.expect("every Binding key comes from KEYS") .expect("every Binding key comes from KEYS")
} }
/// The accelerator in GTK's syntax — `<Ctrl><Shift>f` — which is what a
/// GNOME custom keybinding's `binding` key stores. GTK keyval names are
/// the X11 keysym names, so the keysym column serves both spellings.
pub fn gtk_accelerator(&self) -> String {
let mut out = String::new();
for (held, name) in [
(self.ctrl, "<Ctrl>"),
(self.alt, "<Alt>"),
(self.shift, "<Shift>"),
] {
if held {
out.push_str(name);
}
}
out.push_str(self.row().1);
out
}
/// The trigger in the XDG shortcuts spec's syntax: uppercase modifiers /// The trigger in the XDG shortcuts spec's syntax: uppercase modifiers
/// and an xkbcommon keysym, joined with `+`. /// and an xkbcommon keysym, joined with `+`.
pub fn portal_trigger(&self) -> String { pub fn portal_trigger(&self) -> String {
@ -250,6 +268,17 @@ mod tests {
let binding: Binding = cfg.search_hotkey.parse().expect("the default is valid"); let binding: Binding = cfg.search_hotkey.parse().expect("the default is valid");
assert_eq!(binding.to_string(), "Ctrl+Shift+F"); assert_eq!(binding.to_string(), "Ctrl+Shift+F");
assert_eq!(binding.portal_trigger(), "CTRL+SHIFT+f"); assert_eq!(binding.portal_trigger(), "CTRL+SHIFT+f");
assert_eq!(binding.gtk_accelerator(), "<Ctrl><Shift>f");
}
/// The keysym column doubles as the GTK keyval, so a named key must come
/// out under GTK's name for it, not egui's.
#[test]
fn gtk_accelerators_use_keysym_names() {
let binding: Binding = "Ctrl+Alt+PageUp".parse().unwrap();
assert_eq!(binding.gtk_accelerator(), "<Ctrl><Alt>Prior");
let binding: Binding = "Shift+Enter".parse().unwrap();
assert_eq!(binding.gtk_accelerator(), "<Shift>Return");
} }
#[test] #[test]

View file

@ -30,6 +30,7 @@ mod platform;
mod query_highlight; mod query_highlight;
mod search_tab; mod search_tab;
mod settings_tab; mod settings_tab;
mod shortcut_setup;
mod spotlight; mod spotlight;
#[cfg(test)] #[cfg(test)]
mod test_ui; mod test_ui;
@ -118,8 +119,17 @@ fn main() {
// Losing the race to an instance that came up between the signal // Losing the race to an instance that came up between the signal
// above and here, or a plain second launch: either way the user // above and here, or a plain second launch: either way the user
// asked to see QuickSearch, and there is one to show them. // asked to see QuickSearch, and there is one to show them.
if activate::signal(&Config::config_path()) { //
return; // Retried, not tried once: the winner holds the lock the moment
// `main` reaches it but only listens once eframe's creation
// closure has run, so a `--toggle` landing in that gap would see
// the lock held and no socket. Two seconds outlasts that gap by
// orders of magnitude; a wedged instance still gets the dialog.
for _ in 0..20 {
if activate::signal(&Config::config_path()) {
return;
}
std::thread::sleep(std::time::Duration::from_millis(100));
} }
let who = match pid { let who = match pid {
Some(pid) => format!(" (process {})", pid), Some(pid) => format!(" (process {})", pid),

View file

@ -447,8 +447,8 @@ impl ManageTab {
egui::TextEdit::multiline(&mut self.ext_filter_text) egui::TextEdit::multiline(&mut self.ext_filter_text)
.desired_rows(4) .desired_rows(4)
.desired_width(f32::INFINITY) .desired_width(f32::INFINITY)
.hint_text("#EXAMPLE WHITELISTED FILE EXTENSIONS FOR FULL-TEXT-SEARCH:\n#MOUSE \ .hint_text("#EXAMPLE WHITELISTED FILE EXTENSIONS FOR FULL-TEXT-SEARCH\n#MOUSE \
OVER FOR MORE INFO\n#------------------------------------------------\ntxt\nmd\n \ OVER FOR MORE INFO\ntxt\nmd\n \
pdf # comments allowed\n(none)"), pdf # comments allowed\n(none)"),
) )
.tip(&tips::EXT_WHITELIST); .tip(&tips::EXT_WHITELIST);

View file

@ -293,7 +293,7 @@ impl SettingsTab {
); );
}); });
hotkey_note(ui, &draft.ui.search_hotkey, &current.ui.search_hotkey); hotkey_note(ui, &draft.ui.search_hotkey, &current.ui.search_hotkey);
shortcut_note(ui); shortcut_note(ui, &current.ui.search_hotkey);
ui.separator(); ui.separator();
// Security acts on the live config, not the draft; the KDF // Security acts on the live config, not the draft; the KDF
@ -378,7 +378,7 @@ pub(crate) fn hotkey_edit(
let p = crate::color::palette(ui.visuals().dark_mode); let p = crate::color::palette(ui.visuals().dark_mode);
ui.horizontal(|ui| { ui.horizontal(|ui| {
let label = if *capturing { let label = if *capturing {
"Press a key combination...".to_string() "Press a key combination".to_string()
} else if setting.trim().is_empty() { } else if setting.trim().is_empty() {
"None".to_string() "None".to_string()
} else { } else {
@ -488,26 +488,123 @@ fn hotkey_note(ui: &mut egui::Ui, draft: &str, live: &str) {
/// ///
/// The shortcut above is ours and needs no setup, but it cannot fire while /// The shortcut above is ours and needs no setup, but it cannot fire while
/// QuickSearch is not running — see `crate::activate`. Only the desktop can /// QuickSearch is not running — see `crate::activate`. Only the desktop can
/// bind a key that launches something, so this says what to bind and opens /// bind a key that launches something. Where the desktop's own configuration
/// the place to bind it. Writing the desktop's own configuration instead was /// has a known home for that binding (`crate::shortcut_setup`), one button
/// considered and rejected: it differs per desktop and between versions of /// writes it — into the place the desktop's settings UI lists and edits, so
/// the same one, and a shortcut we wrote and the user cannot see is worse /// it stays the user's to see and change. Everywhere else this says what to
/// than one they created. /// bind and opens the place to bind it.
///
/// `hotkey_setting` is the shortcut in force, which the one-click binding
/// mirrors system-wide.
/// ///
/// Shared with the tour's shortcut page, which puts it under the same /// Shared with the tour's shortcut page, which puts it under the same
/// shortcut button this sentence says is "above". /// shortcut button this sentence says is "above".
pub(crate) fn shortcut_note(ui: &mut egui::Ui) { pub(crate) fn shortcut_note(ui: &mut egui::Ui, hotkey_setting: &str) {
// Tests reach this through the tour's shortcut page; probing the real
// desktop there would make them depend on the machine they run on and
// spawn `gsettings`. Tests that want a desktop pin one through
// `shortcut_note_for`.
let desktop = if cfg!(test) {
crate::shortcut_setup::Desktop::Unsupported
} else {
crate::shortcut_setup::detect()
};
shortcut_note_for(ui, hotkey_setting, desktop);
}
/// One frame's answer from [`crate::shortcut_setup`], cached: `installed`
/// probes the desktop with a subprocess, which must not run per frame.
#[derive(Clone)]
struct SystemShortcutState {
installed: bool,
/// The last install/remove outcome, `(succeeded, what to say)`.
feedback: Option<(bool, String)>,
}
/// The desktop split out of the environment so tests can pick one.
fn shortcut_note_for(ui: &mut egui::Ui, hotkey_setting: &str, desktop: crate::shortcut_setup::Desktop) {
let command = format!("{} --toggle", crate::activate::command_name()); let command = format!("{} --toggle", crate::activate::command_name());
crate::ui_util::stable_section(ui, |ui| { crate::ui_util::stable_section(ui, |ui| {
ui.label( // One-click only with a key to write: an unset or unparseable
egui::RichText::new( // shortcut leaves nothing to bind system-wide.
"The shortcut above works while QuickSearch is open. To have a key \ let binding = crate::hotkey::parse_setting(hotkey_setting).ok().flatten();
start it as well, bind this command in your desktop's keyboard \ let one_click = binding.filter(|_| desktop != crate::shortcut_setup::Desktop::Unsupported);
settings:",
) if let Some(binding) = one_click {
.small() // One id for the settings tab and the tour: they show one fact.
.weak(), let id = egui::Id::new("system-shortcut-state");
); let mut state = ui
.data_mut(|d| d.get_temp::<SystemShortcutState>(id))
.unwrap_or_else(|| SystemShortcutState {
installed: crate::shortcut_setup::installed(),
feedback: None,
});
ui.label(
egui::RichText::new(
"The shortcut above works while QuickSearch is open. Your \
desktop can also bind it to start QuickSearch when it is \
not:",
)
.small()
.weak(),
);
ui.horizontal_wrapped(|ui| {
if state.installed {
ui.label(egui::RichText::new("The system shortcut is set up.").small());
if ui.add(egui::Button::new("Remove").small()).clicked() {
match crate::shortcut_setup::remove() {
Ok(()) => {
state.installed = false;
state.feedback =
Some((true, "System shortcut removed.".to_string()));
}
Err(e) => state.feedback = Some((false, e)),
}
}
} else if ui
.add(egui::Button::new(format!("Set up {} system-wide", binding)).small())
.clicked()
{
match crate::shortcut_setup::install(&binding) {
Ok(()) => {
state.installed = true;
state.feedback = Some((
true,
"Added to your desktop's keyboard shortcuts. If the \
key does not answer right away, it will after the \
next login."
.to_string(),
));
}
Err(e) => state.feedback = Some((false, e)),
}
}
});
if let Some((ok, text)) = &state.feedback {
let rich = egui::RichText::new(text).small();
ui.label(if *ok {
rich.weak()
} else {
rich.color(crate::color::palette(ui.visuals().dark_mode).orange)
});
}
ui.data_mut(|d| d.insert_temp(id, state));
ui.label(
egui::RichText::new("Or bind this command there yourself:")
.small()
.weak(),
);
} else {
ui.label(
egui::RichText::new(
"The shortcut above works while QuickSearch is open. To have a key \
start it as well, bind this command in your desktop's keyboard \
settings:",
)
.small()
.weak(),
);
}
ui.horizontal_wrapped(|ui| { ui.horizontal_wrapped(|ui| {
ui.label(egui::RichText::new(&command).small().monospace()); ui.label(egui::RichText::new(&command).small().monospace());
if ui.add(egui::Button::new("Copy").small()).clicked() { if ui.add(egui::Button::new("Copy").small()).clicked() {
@ -580,30 +677,30 @@ fn security_ui(
} }
ui.horizontal(|ui| { ui.horizontal(|ui| {
if ui if ui
.button("Change password") .button("Change password")
.tip(&tips::CHANGE_PASSWORD) .tip(&tips::CHANGE_PASSWORD)
.clicked() .clicked()
{ {
action = Some(SecurityAction::ChangePassword); action = Some(SecurityAction::ChangePassword);
} }
if ui if ui
.button("Disable protection") .button("Disable protection")
.tip(&tips::DISABLE_PASSWORD) .tip(&tips::DISABLE_PASSWORD)
.clicked() .clicked()
{ {
action = Some(SecurityAction::Disable); action = Some(SecurityAction::Disable);
} }
// The raw key is for someone recovering the file by hand; the
// password controls beside it are for everyone.
if form.advanced
&& ui
.button("Show database key")
.tip(&tips::SHOW_KEY)
.clicked()
{
action = Some(SecurityAction::ShowKey);
}
}); });
// The raw key is for someone recovering the file by hand; the password
// controls above it are for everyone.
if form.advanced
&& ui
.button("Show database key…")
.tip(&tips::SHOW_KEY)
.clicked()
{
action = Some(SecurityAction::ShowKey);
}
let mut remember = current.security.use_keychain; let mut remember = current.security.use_keychain;
if ui if ui
.checkbox(&mut remember, "Remember on this device") .checkbox(&mut remember, "Remember on this device")
@ -615,7 +712,7 @@ fn security_ui(
} else { } else {
ui.label("The index is not encrypted."); ui.label("The index is not encrypted.");
if ui if ui
.button("Enable password protection") .button("Enable password protection")
.tip(&tips::ENABLE_PASSWORD) .tip(&tips::ENABLE_PASSWORD)
.clicked() .clicked()
{ {

View file

@ -559,7 +559,7 @@ fn the_key_button_appears_only_while_the_index_is_encrypted() {
let (_, full) = run_security(&ctx, &cfg, vec![]); let (_, full) = run_security(&ctx, &cfg, vec![]);
assert!( assert!(
painted_text_center(&full, "Show database key").is_none(), painted_text_center(&full, "Show database key").is_none(),
"offered the key of an unencrypted index: {:?}", "offered the key of an unencrypted index: {:?}",
painted_text(&full) painted_text(&full)
); );
@ -567,7 +567,7 @@ fn the_key_button_appears_only_while_the_index_is_encrypted() {
cfg.security.password_protected = true; cfg.security.password_protected = true;
let (_, full) = run_security(&ctx, &cfg, vec![]); let (_, full) = run_security(&ctx, &cfg, vec![]);
assert!( assert!(
painted_text_center(&full, "Show database key").is_some(), painted_text_center(&full, "Show database key").is_some(),
"no key button while encrypted: {:?}", "no key button while encrypted: {:?}",
painted_text(&full) painted_text(&full)
); );
@ -587,7 +587,7 @@ fn clicking_the_key_button_reports_show_key() {
let (quiet, full) = run_security(&ctx, &cfg, vec![]); let (quiet, full) = run_security(&ctx, &cfg, vec![]);
assert!(quiet.is_none(), "reported an action nobody clicked"); assert!(quiet.is_none(), "reported an action nobody clicked");
let target = painted_text_center(&full, "Show database key").expect("no key button"); let target = painted_text_center(&full, "Show database key").expect("no key button");
let (action, _) = run_security(&ctx, &cfg, click_at(target)); let (action, _) = run_security(&ctx, &cfg, click_at(target));
assert_eq!(action, Some(SecurityAction::ShowKey)); assert_eq!(action, Some(SecurityAction::ShowKey));
@ -730,13 +730,20 @@ fn showing_advanced_settings_is_not_an_unsaved_edit() {
} }
/// The panel that tells a user how to get a shortcut that also starts /// The panel that tells a user how to get a shortcut that also starts
/// QuickSearch has to actually show the command they must bind. /// QuickSearch has to actually show the command they must bind — with or
/// without a one-click desktop to lean on.
#[test] #[test]
fn the_shortcut_note_names_the_command_to_bind() { fn the_shortcut_note_names_the_command_to_bind() {
let ctx = crate::test_ui::ctx(); let ctx = crate::test_ui::ctx();
let input = crate::test_ui::raw_input(egui::vec2(700.0, 300.0), vec![]); let input = crate::test_ui::raw_input(egui::vec2(700.0, 300.0), vec![]);
let out = ctx.run(input, |ctx| { let out = ctx.run(input, |ctx| {
egui::CentralPanel::default().show(ctx, |ui| super::shortcut_note(ui)); egui::CentralPanel::default().show(ctx, |ui| {
super::shortcut_note_for(
ui,
"Ctrl+Shift+F",
crate::shortcut_setup::Desktop::Unsupported,
)
});
}); });
let painted = painted_text(&out).join("\n"); let painted = painted_text(&out).join("\n");
assert!( assert!(
@ -745,3 +752,70 @@ fn the_shortcut_note_names_the_command_to_bind() {
); );
assert!(painted.contains("Copy"), "no way to copy it: {painted}"); assert!(painted.contains("Copy"), "no way to copy it: {painted}");
} }
/// On a desktop we can write, the note leads with the one-click button (the
/// probe is skipped by seeding the cached state, so the test stays
/// hermetic), and the manual command stays as the fallback.
#[test]
fn a_supported_desktop_gets_the_one_click_button() {
let ctx = crate::test_ui::ctx();
ctx.data_mut(|d| {
d.insert_temp(
egui::Id::new("system-shortcut-state"),
super::SystemShortcutState {
installed: false,
feedback: None,
},
)
});
let input = crate::test_ui::raw_input(egui::vec2(700.0, 300.0), vec![]);
let out = ctx.run(input, |ctx| {
egui::CentralPanel::default().show(ctx, |ui| {
super::shortcut_note_for(ui, "Ctrl+Shift+F", crate::shortcut_setup::Desktop::Gnome)
});
});
let painted = painted_text(&out).join("\n");
assert!(
painted.contains("Set up Ctrl+Shift+F system-wide"),
"no one-click button: {painted}"
);
assert!(painted.contains("--toggle"), "the fallback vanished: {painted}");
// Already installed: the button flips to removal.
ctx.data_mut(|d| {
d.insert_temp(
egui::Id::new("system-shortcut-state"),
super::SystemShortcutState {
installed: true,
feedback: None,
},
)
});
let input = crate::test_ui::raw_input(egui::vec2(700.0, 300.0), vec![]);
let out = ctx.run(input, |ctx| {
egui::CentralPanel::default().show(ctx, |ui| {
super::shortcut_note_for(ui, "Ctrl+Shift+F", crate::shortcut_setup::Desktop::Gnome)
});
});
let painted = painted_text(&out).join("\n");
assert!(painted.contains("Remove"), "no removal offered: {painted}");
}
/// No usable binding means nothing to write: the one-click flow bows out
/// even on a supported desktop.
#[test]
fn no_binding_means_no_one_click_button() {
let ctx = crate::test_ui::ctx();
let input = crate::test_ui::raw_input(egui::vec2(700.0, 300.0), vec![]);
let out = ctx.run(input, |ctx| {
egui::CentralPanel::default().show(ctx, |ui| {
super::shortcut_note_for(ui, "", crate::shortcut_setup::Desktop::Gnome)
});
});
let painted = painted_text(&out).join("\n");
assert!(
!painted.contains("system-wide"),
"offered to bind nothing: {painted}"
);
assert!(painted.contains("--toggle"), "the manual flow vanished: {painted}");
}

View file

@ -0,0 +1,421 @@
//! Writing the system-wide search shortcut into the desktop's own keyboard
//! configuration, so "bind this command yourself" becomes one click.
//!
//! Auto-writing was once rejected here on the grounds that a shortcut the
//! user cannot see is worse than one they created. The answer is to write it
//! exactly where the desktop's own settings UI lists and edits it — GNOME's
//! custom shortcuts, KDE's global shortcuts — so the binding stays the
//! user's to inspect, change or delete. Desktops without a known home for a
//! binding keep the manual copy-the-command flow in
//! `crate::settings_tab::shortcut_note`.
//!
//! On Windows the closed-app binding lives in the Start-menu `.lnk` the
//! installer creates, so there is nothing to *create* from here; what this
//! module does is keep that `.lnk`'s hotkey in step with the in-app one via
//! [`hotkey_changed`].
use crate::hotkey::Binding;
/// Where a binding can be written. `Unsupported` hides the one-click button
/// and leaves the manual flow.
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub enum Desktop {
Gnome,
Kde,
Unsupported,
}
pub fn detect() -> Desktop {
#[cfg(all(unix, not(target_os = "macos")))]
{
desktop_for(&std::env::var("XDG_CURRENT_DESKTOP").unwrap_or_default())
}
#[cfg(not(all(unix, not(target_os = "macos"))))]
{
Desktop::Unsupported
}
}
/// The mapping itself, split from the environment so it can be tested
/// without one; the same shape as `platform::keyboard_settings_for`.
#[cfg(all(unix, not(target_os = "macos")))]
fn desktop_for(desktops: &str) -> Desktop {
for desktop in desktops.split(':') {
match desktop.to_ascii_uppercase().as_str() {
"GNOME" | "UNITY" => return Desktop::Gnome,
"KDE" => return Desktop::Kde,
_ => {}
}
}
Desktop::Unsupported
}
/// Whether the binding this module writes is currently present. Asks the
/// desktop, so callers should cache rather than poll every frame.
pub fn installed() -> bool {
match detect() {
Desktop::Gnome => gnome::installed(),
Desktop::Kde => kde::installed(),
Desktop::Unsupported => false,
}
}
/// Write `binding` → `quicksearch --toggle` into the desktop's keyboard
/// configuration. Idempotent: a second install rewrites the same entry.
pub fn install(binding: &Binding) -> Result<(), String> {
let command = format!("{} --toggle", crate::activate::command_name());
match detect() {
Desktop::Gnome => gnome::install(binding, &command),
Desktop::Kde => kde::install(binding),
Desktop::Unsupported => Err("this desktop is not supported".to_string()),
}
}
/// Delete the entry [`install`] wrote; a no-op if it is already gone.
pub fn remove() -> Result<(), String> {
match detect() {
Desktop::Gnome => gnome::remove(),
Desktop::Kde => kde::remove(),
Desktop::Unsupported => Err("this desktop is not supported".to_string()),
}
}
/// The in-app shortcut was rebound: keep the system-wide binding in step.
/// Best-effort — a failure is logged, not surfaced, because the change the
/// user asked for (the in-app key) already succeeded.
pub fn hotkey_changed(setting: &str) {
let Ok(Some(binding)) = crate::hotkey::parse_setting(setting) else {
// Cleared or unparseable: leave the system binding alone rather than
// guess; the Settings tab's own controls are the way to remove it.
return;
};
#[cfg(windows)]
{
if let Err(e) = lnk::update_hotkey(&binding) {
quicksearch_core::log_warn!("updating the Start menu shortcut key: {}", e);
}
}
#[cfg(not(windows))]
{
if installed() {
if let Err(e) = install(&binding) {
quicksearch_core::log_warn!("updating the system search shortcut: {}", e);
}
}
}
}
/// Run a program to completion and fail loudly, with its stderr as the why.
#[cfg(all(unix, not(target_os = "macos")))]
fn run(program: &str, args: &[&str]) -> Result<String, String> {
let out = std::process::Command::new(program)
.args(args)
.output()
.map_err(|e| format!("running {}: {}", program, e))?;
if !out.status.success() {
let stderr = String::from_utf8_lossy(&out.stderr);
return Err(format!("{} failed: {}", program, stderr.trim()));
}
Ok(String::from_utf8_lossy(&out.stdout).into_owned())
}
/// GNOME: a custom keybinding under a path of our own. The fixed path is
/// what makes install idempotent and removal exact, and the entry shows up
/// in Settings → Keyboard → Custom Shortcuts under the name given here.
#[cfg(all(unix, not(target_os = "macos")))]
mod gnome {
use super::*;
const LIST_SCHEMA: &str = "org.gnome.settings-daemon.plugins.media-keys";
const LIST_KEY: &str = "custom-keybindings";
const ENTRY_SCHEMA: &str = "org.gnome.settings-daemon.plugins.media-keys.custom-keybinding";
pub(super) const ENTRY_PATH: &str =
"/org/gnome/settings-daemon/plugins/media-keys/custom-keybindings/quicksearch-search/";
pub(super) fn installed() -> bool {
run("gsettings", &["get", LIST_SCHEMA, LIST_KEY])
.map(|list| parse_string_list(&list).iter().any(|p| p == ENTRY_PATH))
.unwrap_or(false)
}
pub(super) fn install(binding: &Binding, command: &str) -> Result<(), String> {
let entry = format!("{}:{}", ENTRY_SCHEMA, ENTRY_PATH);
for (key, value) in [
("name", "QuickSearch".to_string()),
("command", command.to_string()),
("binding", binding.gtk_accelerator()),
] {
run("gsettings", &["set", &entry, key, &value])?;
}
// The entry only takes effect once its path is on the list; last, so
// a failure above cannot leave a listed entry with no command.
let list = run("gsettings", &["get", LIST_SCHEMA, LIST_KEY])?;
let mut paths = parse_string_list(&list);
if !paths.iter().any(|p| p == ENTRY_PATH) {
paths.push(ENTRY_PATH.to_string());
let list = format_string_list(&paths);
run("gsettings", &["set", LIST_SCHEMA, LIST_KEY, &list])?;
}
Ok(())
}
pub(super) fn remove() -> Result<(), String> {
let list = run("gsettings", &["get", LIST_SCHEMA, LIST_KEY])?;
let paths: Vec<String> = parse_string_list(&list)
.into_iter()
.filter(|p| p != ENTRY_PATH)
.collect();
let list = format_string_list(&paths);
run("gsettings", &["set", LIST_SCHEMA, LIST_KEY, &list])?;
let entry = format!("{}:{}", ENTRY_SCHEMA, ENTRY_PATH);
run("gsettings", &["reset-recursively", &entry])?;
Ok(())
}
}
/// The GVariant `as` (array of strings) spelling `gsettings get` prints and
/// `gsettings set` accepts: `['a', 'b']`, or `@as []` when empty.
///
/// The parser accepts exactly what gsettings emits — single-quoted strings
/// with `\'` and `\\` escapes — and drops anything malformed rather than
/// guessing: a path we misread would be written back verbatim into the
/// user's configuration.
#[cfg(all(unix, not(target_os = "macos")))]
fn parse_string_list(raw: &str) -> Vec<String> {
let mut paths = Vec::new();
let mut current = None;
let mut escaped = false;
for ch in raw.chars() {
match current.as_mut() {
None => {
if ch == '\'' {
current = Some(String::new());
}
}
Some(path) => {
if escaped {
path.push(ch);
escaped = false;
} else if ch == '\\' {
escaped = true;
} else if ch == '\'' {
paths.push(current.take().expect("current is Some in this arm"));
} else {
path.push(ch);
}
}
}
}
paths
}
#[cfg(all(unix, not(target_os = "macos")))]
fn format_string_list(paths: &[String]) -> String {
if paths.is_empty() {
// A bare `[]` has no type; this is the empty list gsettings prints.
return "@as []".to_string();
}
let quoted: Vec<String> = paths
.iter()
.map(|p| format!("'{}'", p.replace('\\', "\\\\").replace('\'', "\\'")))
.collect();
format!("[{}]", quoted.join(", "))
}
/// KDE: the global-shortcuts entry for the `Search` action that
/// `packaging/quicksearch.desktop` declares (`Exec=quicksearch --toggle`).
/// kglobalaccel launches desktop-file actions itself, so no command is
/// written here — only the key, in the file KDE's own Shortcuts settings
/// page reads and edits.
#[cfg(all(unix, not(target_os = "macos")))]
mod kde {
use super::*;
const FILE: &str = "kglobalshortcutsrc";
const GROUP: &str = "quicksearch.desktop";
/// Plasma 6's tool first; 5's second. The first present wins.
fn config_tool(names: [&'static str; 2]) -> &'static str {
let on_path = |name: &str| {
std::env::var_os("PATH").is_some_and(|path| {
std::env::split_paths(&path).any(|dir| dir.join(name).is_file())
})
};
if on_path(names[0]) {
names[0]
} else {
names[1]
}
}
/// The entry format is `active,default,description`.
pub(super) fn entry(binding: &Binding) -> String {
format!("{},none,Search", binding)
}
pub(super) fn installed() -> bool {
let tool = config_tool(["kreadconfig6", "kreadconfig5"]);
run(tool, &["--file", FILE, "--group", GROUP, "--key", "Search"])
.map(|out| {
let active = out.trim().split(',').next().unwrap_or("");
!active.is_empty() && active != "none"
})
.unwrap_or(false)
}
pub(super) fn install(binding: &Binding) -> Result<(), String> {
let tool = config_tool(["kwriteconfig6", "kwriteconfig5"]);
let entry = entry(binding);
for (key, value) in [("_k_friendly_name", "QuickSearch"), ("Search", &entry)] {
run(
tool,
&["--file", FILE, "--group", GROUP, "--key", key, value],
)?;
}
reload();
Ok(())
}
pub(super) fn remove() -> Result<(), String> {
let tool = config_tool(["kwriteconfig6", "kwriteconfig5"]);
for key in ["Search", "_k_friendly_name"] {
run(
tool,
&["--file", FILE, "--group", GROUP, "--key", key, "--delete"],
)?;
}
reload();
Ok(())
}
/// Ask kglobalaccel to re-read its file. Best-effort: without it the
/// binding takes effect at the next login, which install's caller says.
fn reload() {
for qdbus in ["qdbus6", "qdbus"] {
if run(
qdbus,
&[
"org.kde.kglobalaccel",
"/kglobalaccel",
"org.kde.KGlobalAccel.reloadConfig",
],
)
.is_ok()
{
return;
}
}
}
}
/// Windows: the hotkey field of the Start-menu `.lnk` is the closed-app
/// binding (see `packaging/quicksearch.nsi`), rewritten through
/// `WScript.Shell` — whose `Hotkey` property takes exactly the
/// `Ctrl+Shift+F` spelling [`Binding`] displays — rather than through a COM
/// vtable of our own.
#[cfg(windows)]
mod lnk {
use super::*;
use std::path::PathBuf;
fn start_menu_lnk(env: &str) -> Option<PathBuf> {
let base = std::env::var_os(env)?;
let path = PathBuf::from(base).join(r"Microsoft\Windows\Start Menu\Programs\QuickSearch.lnk");
path.is_file().then_some(path)
}
pub(super) fn update_hotkey(binding: &Binding) -> Result<(), String> {
// The installer elevates and writes the all-users Start menu; a
// per-user one is checked first because that is the one this
// unelevated process can rewrite.
let lnk = start_menu_lnk("APPDATA")
.or_else(|| start_menu_lnk("ProgramData"))
.ok_or("no Start menu shortcut exists; re-run the installer")?;
let script = format!(
"$s = (New-Object -ComObject WScript.Shell).CreateShortcut('{}'); \
$s.Hotkey = '{}'; $s.Save()",
lnk.display().to_string().replace('\'', "''"),
binding,
);
let out = std::process::Command::new("powershell")
.args(["-NoProfile", "-NonInteractive", "-Command", &script])
.output()
.map_err(|e| format!("running powershell: {}", e))?;
if !out.status.success() {
let stderr = String::from_utf8_lossy(&out.stderr);
// The common failure: the .lnk is the elevated installer's.
return Err(format!(
"could not rewrite {} ({}); if QuickSearch was installed for \
all users, re-run the installer to change the key",
lnk.display(),
stderr.trim(),
));
}
Ok(())
}
}
#[cfg(test)]
mod tests {
use super::*;
#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn the_desktop_is_detected_from_the_list_case_insensitively() {
assert_eq!(desktop_for("GNOME"), Desktop::Gnome);
assert_eq!(desktop_for("ubuntu:GNOME"), Desktop::Gnome);
assert_eq!(desktop_for("kde"), Desktop::Kde);
assert_eq!(desktop_for("Unity"), Desktop::Gnome);
assert_eq!(desktop_for(""), Desktop::Unsupported);
assert_eq!(desktop_for("i3:sway"), Desktop::Unsupported);
}
/// Round trip through the exact spellings gsettings prints.
#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn the_gvariant_list_round_trips() {
assert_eq!(parse_string_list("@as []"), Vec::<String>::new());
assert_eq!(parse_string_list("[]"), Vec::<String>::new());
let two = parse_string_list("['/a/path/', '/b/path/']");
assert_eq!(two, ["/a/path/", "/b/path/"]);
assert_eq!(format_string_list(&two), "['/a/path/', '/b/path/']");
assert_eq!(format_string_list(&[]), "@as []");
}
/// A path with a quote in it must survive both directions, or the write
/// back would corrupt the user's other bindings.
#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn escaped_quotes_round_trip() {
let paths = vec!["/it's/".to_string(), "/back\\slash/".to_string()];
let formatted = format_string_list(&paths);
assert_eq!(parse_string_list(&formatted), paths);
}
/// Malicious or truncated gsettings output must never panic; at worst it
/// yields fewer paths.
#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn garbage_lists_parse_to_something_harmless() {
for garbage in ["", "[", "['unterminated", "not a list", "['a'", "\\"] {
let _ = parse_string_list(garbage);
}
}
#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn the_kde_entry_carries_the_binding_first() {
let binding: Binding = "Ctrl+Shift+F".parse().unwrap();
assert_eq!(kde::entry(&binding), "Ctrl+Shift+F,none,Search");
}
/// The fixed GNOME path is load-bearing twice over: idempotence and
/// exact removal both key on it.
#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn the_gnome_entry_path_is_fixed_and_well_formed() {
assert!(gnome::ENTRY_PATH.starts_with('/'));
assert!(gnome::ENTRY_PATH.ends_with('/'));
assert!(gnome::ENTRY_PATH.contains("quicksearch"));
}
}

View file

@ -506,9 +506,9 @@ pub static REMEMBER_KEYCHAIN: Tip = Tip {
body: "Hands the key to the password store your system already has, such \ body: "Hands the key to the password store your system already has, such \
as GNOME Keyring, KWallet, or Windows Credential Manager, so that \ as GNOME Keyring, KWallet, or Windows Credential Manager, so that \
QuickSearch can unlock the index without asking at startup.\n\n\ QuickSearch can unlock the index without asking at startup.\n\n\
The password itself is never stored, only the key worked out from \ The password itself is never stored, only key securely created from \
it, and only on this machine. Off, you type the password each time \ it, and only on this machine. When off you must type the password each \
QuickSearch starts.", time QuickSearch starts.",
examples: &[], examples: &[],
caution: None, caution: None,
}; };

View file

@ -345,6 +345,9 @@ fn note(ui: &egui::Ui, text: impl Into<egui::RichText>) -> egui::RichText {
struct Live<'a> { struct Live<'a> {
/// `ctx.zoom_factor()`, read before the window is laid out. /// `ctx.zoom_factor()`, read before the window is laid out.
zoom: f32, zoom: f32,
/// The slider's position while it differs from `zoom` — chosen but not
/// yet applied. `None` once Apply is clicked or nothing is pending.
staged_scale: &'a mut Option<f32>,
/// The shortcut in force, as the config spells it. /// The shortcut in force, as the config spells it.
hotkey: &'a str, hotkey: &'a str,
capturing_hotkey: &'a mut bool, capturing_hotkey: &'a mut bool,
@ -357,27 +360,24 @@ fn extra_ui(ui: &mut egui::Ui, extra: Extra, live: &mut Live, actions: &mut Tour
ui.separator(); ui.separator();
match extra { match extra {
Extra::Scale => { Extra::Scale => {
let mut scale = live.zoom; // Staged until Apply, like the Settings tab's slider: applying
// mid-drag rescales the slider under the pointer, so the handle
// chases its own tail and the value cannot be chosen.
let mut scale = live.staged_scale.unwrap_or(live.zoom);
ui.horizontal(|ui| { ui.horizontal(|ui| {
ui.label("UI scale"); ui.label("UI scale");
// Scoped to this row, which is the whole of its use. // Scoped to this row, which is the whole of its use.
ui.spacing_mut().slider_width = 220.0; ui.spacing_mut().slider_width = 220.0;
let slider = ui.add( ui.add(
egui::Slider::new(&mut scale, crate::app::SCALE_RANGE) egui::Slider::new(&mut scale, crate::app::SCALE_RANGE)
.step_by(0.05) .step_by(0.05)
.fixed_decimals(2), .fixed_decimals(2),
); );
// Applied on every frame it moves, saved once it settles: a let pending = (scale - live.zoom).abs() > f32::EPSILON;
// drag would otherwise rewrite the config file dozens of *live.staged_scale = pending.then_some(scale);
// times on its way across. The release frame is not itself a if ui.add_enabled(pending, egui::Button::new("Apply")).clicked() {
// change — the value stopped moving — so it is asked about
// separately, and carries the value with it so the app has
// one thing to act on.
let moved = slider.changed();
let settled = slider.drag_stopped() || (moved && !slider.dragged());
if moved || settled {
actions.set_scale = Some(scale); actions.set_scale = Some(scale);
actions.save_scale = settled; *live.staged_scale = None;
} }
}); });
ui.label(note( ui.label(note(
@ -399,7 +399,7 @@ fn extra_ui(ui: &mut egui::Ui, extra: Extra, live: &mut Live, actions: &mut Tour
if setting != live.hotkey { if setting != live.hotkey {
actions.set_hotkey = Some(setting); actions.set_hotkey = Some(setting);
} }
crate::settings_tab::shortcut_note(ui); crate::settings_tab::shortcut_note(ui, live.hotkey);
} }
} }
} }
@ -453,10 +453,10 @@ pub struct TourActions {
pub goto_tab: Option<Tab>, pub goto_tab: Option<Tab>,
pub set_query: Option<String>, pub set_query: Option<String>,
pub focus_search: bool, pub focus_search: bool,
/// A new UI scale from the welcome page's slider, to apply live. /// A new UI scale from the welcome page's slider, applied and saved on
/// its Apply button — never mid-drag, which would rescale the slider
/// under the pointer.
pub set_scale: Option<f32>, pub set_scale: Option<f32>,
/// The drag ended, so the scale above is worth writing to the config.
pub save_scale: bool,
/// A shortcut captured on the shortcut page, to register and save. /// A shortcut captured on the shortcut page, to register and save.
pub set_hotkey: Option<String>, pub set_hotkey: Option<String>,
} }
@ -473,6 +473,8 @@ pub struct Tutorial {
/// The shortcut button is armed and the next key combination is the /// The shortcut button is armed and the next key combination is the
/// answer — the Settings tab's own capture, and its own flag. /// answer — the Settings tab's own capture, and its own flag.
capturing_hotkey: bool, capturing_hotkey: bool,
/// The welcome page's scale slider, between moving and Apply.
staged_scale: Option<f32>,
} }
impl Tutorial { impl Tutorial {
@ -483,6 +485,7 @@ impl Tutorial {
typing: None, typing: None,
moved: false, moved: false,
capturing_hotkey: false, capturing_hotkey: false,
staged_scale: None,
} }
} }
@ -526,6 +529,7 @@ impl Tutorial {
// Lifted out of `self` for the window's closure, and put back after: // Lifted out of `self` for the window's closure, and put back after:
// the closure already holds the page and the actions. // the closure already holds the page and the actions.
let mut capturing = self.capturing_hotkey; let mut capturing = self.capturing_hotkey;
let mut staged_scale = self.staged_scale;
let mut dismissed = false; let mut dismissed = false;
let mut window = egui::Window::new(page.title) let mut window = egui::Window::new(page.title)
.id(egui::Id::new(WINDOW_ID)) .id(egui::Id::new(WINDOW_ID))
@ -555,6 +559,7 @@ impl Tutorial {
if let Some(extra) = page.extra { if let Some(extra) = page.extra {
let mut live = Live { let mut live = Live {
zoom, zoom,
staged_scale: &mut staged_scale,
hotkey, hotkey,
capturing_hotkey: &mut capturing, capturing_hotkey: &mut capturing,
}; };
@ -604,6 +609,7 @@ impl Tutorial {
self.moved = true; self.moved = true;
} }
self.capturing_hotkey = capturing; self.capturing_hotkey = capturing;
self.staged_scale = staged_scale;
// The widgets this page names, in the colours its keywords were given. // The widgets this page names, in the colours its keywords were given.
let dark_mode = ctx.style().visuals.dark_mode; let dark_mode = ctx.style().visuals.dark_mode;

View file

@ -22,6 +22,7 @@ fn at(page: usize) -> Tutorial {
typing: None, typing: None,
moved: false, moved: false,
capturing_hotkey: false, capturing_hotkey: false,
staged_scale: None,
} }
} }
@ -34,6 +35,7 @@ fn entering(page: usize) -> Tutorial {
typing: None, typing: None,
moved: false, moved: false,
capturing_hotkey: false, capturing_hotkey: false,
staged_scale: None,
} }
} }
@ -59,7 +61,6 @@ fn merge(a: TourActions, b: TourActions) -> TourActions {
set_query: b.set_query.or(a.set_query), set_query: b.set_query.or(a.set_query),
focus_search: a.focus_search || b.focus_search, focus_search: a.focus_search || b.focus_search,
set_scale: b.set_scale.or(a.set_scale), set_scale: b.set_scale.or(a.set_scale),
save_scale: a.save_scale || b.save_scale,
set_hotkey: b.set_hotkey.or(a.set_hotkey), set_hotkey: b.set_hotkey.or(a.set_hotkey),
} }
} }
@ -302,9 +303,11 @@ fn drag(
} }
/// The whole point of putting it on the first page: someone who cannot read /// The whole point of putting it on the first page: someone who cannot read
/// the window can fix that without finding the Settings tab first. /// the window can fix that without finding the Settings tab first. Staged
/// until Apply: applying mid-drag would rescale the slider under the
/// pointer, and the handle would chase its own tail.
#[test] #[test]
fn the_welcome_page_slider_sets_the_ui_scale() { fn the_welcome_page_slider_applies_only_on_its_button() {
let ctx = crate::test_ui::ctx(); let ctx = crate::test_ui::ctx();
let scale_page = page_with(Extra::Scale); let scale_page = page_with(Extra::Scale);
let mut tour = at(scale_page); let mut tour = at(scale_page);
@ -316,28 +319,36 @@ fn the_welcome_page_slider_sets_the_ui_scale() {
.1; .1;
// The rail runs to the right of its label, on the same row. // The rail runs to the right of its label, on the same row.
let from = egui::pos2(label.right() + 30.0, label.center().y); let from = egui::pos2(label.right() + 30.0, label.center().y);
let [_, moved, release] = drag(&ctx, &mut tour, from, from + egui::vec2(120.0, 0.0)); let [press, moved, release] = drag(&ctx, &mut tour, from, from + egui::vec2(120.0, 0.0));
for (what, actions) in [("press", &press), ("move", &moved), ("release", &release)] {
let dragged = moved assert_eq!(
.set_scale actions.set_scale, None,
.expect("dragging the slider changed nothing"); "the {what} applied the scale without Apply being clicked"
);
}
let staged = tour.staged_scale.expect("the drag staged nothing");
assert!( assert!(
crate::app::SCALE_RANGE.contains(&dragged), crate::app::SCALE_RANGE.contains(&staged),
"{dragged} is outside the range the slider offers" "{staged} is outside the range the slider offers"
); );
assert_ne!(dragged, 1.0, "the drag did not move the value"); assert_ne!(staged, 1.0, "the drag did not move the value");
assert!(!moved.save_scale, "the config was written mid-drag");
assert_eq!(
release.set_scale,
Some(dragged),
"the release did not hand the settled value over to be saved"
);
assert!(release.save_scale, "the drag ended without being saved");
// And nothing happens on a frame nobody touched it. // The staged value survives idle frames, then Apply hands it over once.
let (_, quiet) = pass(&ctx, &mut tour, Vec::new(), 2.0); let (out, quiet) = pass(&ctx, &mut tour, Vec::new(), 2.0);
assert_eq!(quiet.set_scale, None); assert_eq!(quiet.set_scale, None);
assert!(!quiet.save_scale); assert_eq!(tour.staged_scale, Some(staged));
let apply = painted(&out)
.into_iter()
.find(|(text, _)| text == "Apply")
.expect("no Apply button beside the slider")
.1;
let (_, applied) = pass(&ctx, &mut tour, click_at(apply.center()), 2.1);
assert_eq!(
applied.set_scale,
Some(staged),
"Apply did not hand the staged value over"
);
assert_eq!(tour.staged_scale, None, "Apply left the value staged");
} }
/// The slider shows the size the window is already at — the config's, or /// The slider shows the size the window is already at — the config's, or

View file

@ -56,14 +56,18 @@ impl Gate {
/// it or a `--toggle` process relayed the desktop's. Handled here because /// it or a `--toggle` process relayed the desktop's. Handled here because
/// while locked the unlock screen *is* the window. /// while locked the unlock screen *is* the window.
fn handle_activation(&mut self, ctx: &egui::Context, frame: &eframe::Frame) { fn handle_activation(&mut self, ctx: &egui::Context, frame: &eframe::Frame) {
if let Gate::Running(app) = self {
// The shortcut must not reshuffle the window under a key capture.
// The flag is left set, not consumed: capture ends on a key
// event, which repaints, and the press is acted on that frame.
if app.capturing_hotkey() {
return;
}
}
if !crate::activate::take_pending() { if !crate::activate::take_pending() {
return; return;
} }
if let Gate::Running(app) = self { if let Gate::Running(app) = self {
// The shortcut must not reshuffle the window under a key capture.
if app.capturing_hotkey() {
return;
}
app.activate_search(ctx); app.activate_search(ctx);
} }
crate::activate::raise(ctx, frame); crate::activate::raise(ctx, frame);

View file

@ -174,21 +174,23 @@ Section "Start Menu shortcut" SecStartMenu
CreateShortcut "$SMPROGRAMS\${APP}.lnk" "$INSTDIR\quicksearch.exe" "--toggle" "$INSTDIR\quicksearch.ico" CreateShortcut "$SMPROGRAMS\${APP}.lnk" "$INSTDIR\quicksearch.exe" "--toggle" "$INSTDIR\quicksearch.ico"
SectionEnd SectionEnd
Section "Search hotkey (Ctrl+Alt+F)" SecHotkey Section "Search hotkey (Ctrl+Shift+F)" SecHotkey
; The .lnk "shortcut key" field is the only thing on Windows that binds a ; The .lnk "shortcut key" field is the only thing on Windows that binds a
; key to a command, and it is what makes the shortcut work while ; key to a command, and it is what makes the shortcut work while
; QuickSearch is closed - nothing an application registers for itself can ; QuickSearch is closed - nothing an application registers for itself can
; fire when it is not running. Windows only honours the field on a ; fire when it is not running. Windows only honours the field on a
; shortcut in the Start menu or on the desktop, and only for combinations ; shortcut in the Start menu or on the desktop. Modifier combinations
; including Ctrl+Alt, which is why this is Ctrl+Alt+F and not the ; like Ctrl+Shift are accepted as-is; only a bare key gets Ctrl+Alt added
; Ctrl+Shift+F the Settings tab offers. The in-application shortcut takes ; for it. Ctrl+Shift+F matches the in-application default, which answers
; any combination but only answers while the window is already open, so ; the key instantly while the window is open; this .lnk binding is the
; the two are complementary rather than duplicates. ; slower launch path Explorer takes when it is not. Changing the shortcut
; on the Settings tab rewrites this .lnk to match (per-user installs
; only; this all-users file needs elevation).
; ;
; Rewrites the same shortcut the section above creates: NSIS cannot add a ; Rewrites the same shortcut the section above creates: NSIS cannot add a
; hotkey to an existing .lnk, and creating it twice is harmless. ; hotkey to an existing .lnk, and creating it twice is harmless.
CreateShortcut "$SMPROGRAMS\${APP}.lnk" "$INSTDIR\quicksearch.exe" "--toggle" \ CreateShortcut "$SMPROGRAMS\${APP}.lnk" "$INSTDIR\quicksearch.exe" "--toggle" \
"$INSTDIR\quicksearch.ico" 0 SW_SHOWNORMAL ALT|CONTROL|F \ "$INSTDIR\quicksearch.ico" 0 SW_SHOWNORMAL CONTROL|SHIFT|F \
"Search your files with ${APP}" "Search your files with ${APP}"
SectionEnd SectionEnd
@ -212,9 +214,8 @@ SectionEnd
!insertmacro MUI_DESCRIPTION_TEXT ${SecStartMenu} \ !insertmacro MUI_DESCRIPTION_TEXT ${SecStartMenu} \
"Add ${APP} to the Start menu for all users." "Add ${APP} to the Start menu for all users."
!insertmacro MUI_DESCRIPTION_TEXT ${SecHotkey} \ !insertmacro MUI_DESCRIPTION_TEXT ${SecHotkey} \
"Press Ctrl+Alt+F anywhere to search, starting ${APP} if it is not \ "Press Ctrl+Shift+F anywhere to search, starting ${APP} if it is not \
already running. Windows allows this only on Ctrl+Alt combinations; \ already running. The Settings tab can rebind it."
the Settings tab has one that takes any keys while ${APP} is open."
!insertmacro MUI_DESCRIPTION_TEXT ${SecDesktop} \ !insertmacro MUI_DESCRIPTION_TEXT ${SecDesktop} \
"Add a ${APP} shortcut to the desktop." "Add a ${APP} shortcut to the desktop."
!insertmacro MUI_FUNCTION_DESCRIPTION_END !insertmacro MUI_FUNCTION_DESCRIPTION_END