Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions PKGBUILD
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ package() {
nextsync-activity-ok
nextsync-activity-warning
nextsync-conflict-warning
nextsync-menu-info
nextsync-menu-log
nextsync-menu-open
nextsync-menu-quit
Expand Down
1 change: 1 addition & 0 deletions data/icons/nextsync-menu-info.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion data/icons/nextsync-menu-log.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion data/icons/nextsync-menu-open.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion data/icons/nextsync-menu-quit.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
84 changes: 75 additions & 9 deletions src/core/account_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -785,16 +785,31 @@ impl AccountRuntime {
// drive their own triggers). Without this the app sits idle until an
// event or interval fires, so a fresh install never compares the
// trees and the not-yet-synchronized state sticks until a manual run.
let schedulers: Vec<_> = self
.folders
.values()
.map(|folder| folder.scheduler())
.collect();
for scheduler in schedulers {
scheduler.request(Trigger::Startup);
//
// Issue #154: request the runs in the configured folder order, not
// the HashMap's (random) iteration order. The shared one-at-a-time
// permit wakes its waiters FIFO, so the request order IS the run
// order; following the configuration keeps the startup backlog
// stable and matching the folder list the window shows.
for runtime in self.ordered_folders() {
runtime.scheduler().request(Trigger::Startup);
}
}

/// The folder runtimes in the account's configured order.
///
/// `self.folders` is a HashMap, so its iteration order is random per
/// process; any fan-out whose order is user-visible (the startup sync
/// requests and the watcher target list, issue #154) must follow the
/// configuration instead.
fn ordered_folders(&self) -> Vec<&FolderRuntime> {
self.account
.folders
.iter()
.filter_map(|folder| self.folders.get(&folder.id))
.collect()
}

/// Wire the GLib main-loop consumers for every folder runtime: the local
/// filesystem watcher and the live progress forwarder.
///
Expand Down Expand Up @@ -863,10 +878,14 @@ impl AccountRuntime {

/// Refresh the shared scheduler list watched by the system callbacks from
/// the current folder runtimes (folders can be added or removed at runtime).
///
/// The resume and remote-push callbacks request one sync per folder
/// through this list, so it follows the configured folder order (issue
/// #154), exactly like the startup fan-out.
fn sync_targets(&self) {
let schedulers: Vec<Scheduler> = self
.folders
.values()
.ordered_folders()
.iter()
.map(|folder| folder.scheduler())
.collect();
self.targets.borrow_mut().schedulers = schedulers;
Expand Down Expand Up @@ -1487,6 +1506,53 @@ mod tests {
assert_eq!(runtime.state().snapshot().state, AppState::Unconfigured);
}

/// Issue #154: the startup fan-out must follow the configured folder
/// order. `self.folders` is a HashMap (random iteration order per
/// process), so the startup requests used to go out in a different order
/// on every launch and the shared permit's FIFO ran the backlog in that
/// random order; the window lists folders in configuration order, hence
/// the visible hopping.
#[test]
fn startup_requests_follow_the_configured_folder_order() {
let source = fake_source();
let mut account = sample_account(true);
account.folders = vec![
FolderConfig {
id: "f-one".to_string(),
local_root: "/tmp/nsync-order-1".to_string(),
remote_path: "/one".to_string(),
space_id: None,
size_confirmed: false,
},
FolderConfig {
id: "f-two".to_string(),
local_root: "/tmp/nsync-order-2".to_string(),
remote_path: "/two".to_string(),
space_id: None,
size_confirmed: false,
},
FolderConfig {
id: "f-three".to_string(),
local_root: "/tmp/nsync-order-3".to_string(),
remote_path: "/three".to_string(),
space_id: None,
size_confirmed: false,
},
];
let mut runtime =
AccountRuntime::new(account, NetworkConfig::default(), source, None, false);
runtime.start_without_watchers();
assert_eq!(runtime.folders().len(), 3);
// The same ordering source `mount_watchers` uses for the startup
// Trigger::Startup requests: configuration order, never HashMap order.
let order: Vec<&str> = runtime
.ordered_folders()
.iter()
.map(|runtime| runtime.folder.id.as_str())
.collect();
assert_eq!(order, vec!["f-one", "f-two", "f-three"]);
}

#[test]
fn manager_starts_every_account_and_removes_one() {
let source = fake_source();
Expand Down
43 changes: 43 additions & 0 deletions src/core/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1247,6 +1247,49 @@ mod tests {
assert_eq!(runner_second.0.borrow().start_calls, 1);
}

/// Issue #154: the startup backlog runs strictly one folder at a time in
/// the exact order the startup triggers were requested. Every scheduler
/// defers its start through one idle hop (dispatched FIFO, like the GLib
/// equal-priority idle sources in production) and the shared permit
/// wakes its waiters oldest-first, so the request order IS the run order
/// across all accounts and folders.
#[test]
fn startup_backlog_runs_strictly_in_request_order() {
let permit = SyncPermit::try_new(1).unwrap();
let (first, source_first, runner_first) = make_scheduler(Some(permit.clone()));
let (second, source_second, runner_second) = make_scheduler(Some(permit.clone()));
let (third, source_third, runner_third) = make_scheduler(Some(permit));

// The startup fan-out requests one run per folder; the idles then
// fire in request order.
first.request(Trigger::Startup);
second.request(Trigger::Startup);
third.request(Trigger::Startup);
run_idle(&source_first);
run_idle(&source_second);
run_idle(&source_third);

// Exactly one run started; the rest of the backlog waits on the
// permit, oldest request first.
assert_eq!(runner_first.0.borrow().start_calls, 1);
assert_eq!(runner_second.0.borrow().start_calls, 0);
assert_eq!(runner_third.0.borrow().start_calls, 0);

// Finishing a run wakes only the oldest waiter: the second folder
// runs next, never the third.
finish(&runner_first, SyncOutcome::Success);
assert_eq!(source_second.borrow().pending(), 1);
assert_eq!(source_third.borrow().pending(), 0);
run_idle(&source_second);
assert_eq!(runner_second.0.borrow().start_calls, 1);
assert_eq!(runner_third.0.borrow().start_calls, 0);

finish(&runner_second, SyncOutcome::Success);
assert_eq!(source_third.borrow().pending(), 1);
run_idle(&source_third);
assert_eq!(runner_third.0.borrow().start_calls, 1);
}

#[test]
fn external_engine_on_the_folder_aborts_the_run_with_a_clear_error() {
let (scheduler, source, runner) = make_scheduler(None);
Expand Down
12 changes: 12 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,18 @@ fn main() {
}
}
})),
// Issue #155: the same About dialog the hamburger menu opens.
// With minimize-to-tray the main window may be hidden; the
// dialog is transient for it and GTK still presents it on its
// own, so no standalone fallback is needed.
open_about: Rc::new({
let weak = weak.clone();
move || {
if let Some(main) = weak.upgrade() {
main.borrow_mut().show_about();
}
}
}),
pause_all: Rc::new({
let weak = weak.clone();
move |paused| {
Expand Down
49 changes: 37 additions & 12 deletions src/ui/tray.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ pub enum TrayAction {
Conflicts,
/// Pause or resume every account at once (issue #42).
PauseAll(bool),
/// Present the About dialog (issue #155).
About,
/// Quit the application.
Quit,
}
Expand All @@ -73,6 +75,8 @@ pub struct TrayCallbacks {
pub pause_all: Rc<dyn Fn(bool)>,
/// Whether every account is currently paused (drives the menu label).
pub all_paused: Rc<dyn Fn() -> bool>,
/// Present the About dialog (issue #155).
pub open_about: Rc<dyn Fn()>,
/// Quit the application.
pub quit: Rc<dyn Fn()>,
}
Expand Down Expand Up @@ -114,8 +118,8 @@ pub fn status_icon_key_to_name(icon_key: &str) -> &'static str {
}
}

/// Number of items in the tray menu (Open, Log, Quit).
pub const MENU_ITEM_COUNT: usize = 3;
/// Number of items in the tray menu (Open, Log, About, Quit).
pub const MENU_ITEM_COUNT: usize = 4;

/// The StatusNotifier item. Only `Send` data lives here, satisfying the
/// `ksni::Tray` bound; user actions leave through the [`TrayAction`] channel.
Expand Down Expand Up @@ -151,18 +155,20 @@ impl TrayItem {
self.all_paused = paused;
}

/// The menu items: Open, Settings, Conflicts (when wired) and Quit,
/// following the v0.4.0 tray (`_layout_data` item ids 1, 7, 8 plus the
/// conflicts entry `application.py` wires via `open_conflicts`).
/// The menu items: Open, Log (when wired), About and Quit, following
/// the v0.4.0 tray (`_layout_data` item ids 1, 7, 8 plus the conflicts
/// entry `application.py` wires via `open_conflicts`); About joins from
/// the window's hamburger menu (issue #155).
///
/// The callbacks run on the ksni service thread, so they only post a
/// [`TrayAction`] with `try_send` (async-channel 2.x `Sender::send` is an
/// async fn and would need an executor to make progress).
fn build_menu(&self) -> Vec<MenuItem<Self>> {
// Issue #84: Settings and Pause Everything left the tray menu; both
// live in the main window, one Open click away. The menu keeps Open,
// Log (when wired) and Quit.
// Log (when wired), About (issue #155) and Quit.
let open = self.actions.clone();
let about = self.actions.clone();
let quit = self.actions.clone();
let mut items: Vec<MenuItem<Self>> = vec![StandardItem {
label: t("Open NextSync").into(),
Expand Down Expand Up @@ -201,6 +207,19 @@ impl TrayItem {
}
.into(),
);
items.push(
StandardItem {
// Same dialog (and same Lucide info icon) as the window's
// hamburger menu (issue #155); the item closes the menu.
label: t("About").into(),
icon_name: "nextsync-menu-info".into(),
activate: Box::new(move |_this: &mut Self| {
let _ = about.try_send(TrayAction::About);
}),
..Default::default()
}
.into(),
);
items
}
}
Expand Down Expand Up @@ -291,7 +310,7 @@ impl Tray {
let _ = self.handle.update(|item| item.set_all_paused(paused));
}

/// Number of menu items (Open, Settings, [Conflicts], Quit).
/// Number of menu items (Open, [Log], About, Quit).
pub fn menu_items(&self) -> usize {
self.handle
.update(|item| item.build_menu().len())
Expand All @@ -313,6 +332,7 @@ async fn dispatch(receiver: async_channel::Receiver<TrayAction>, callbacks: Tray
TrayAction::PauseAll(paused) => {
(callbacks.pause_all)(paused);
}
TrayAction::About => (callbacks.open_about)(),
TrayAction::Quit => (callbacks.quit)(),
}
}
Expand All @@ -330,7 +350,7 @@ mod tests {
}

#[test]
fn menu_has_three_items_when_conflicts_is_wired() {
fn menu_has_the_expected_items_when_conflicts_is_wired() {
set_locale(Locale::English);
let (item, _rx) = item_with(AppState::IdleOk);
let menu = item.build_menu();
Expand All @@ -342,8 +362,9 @@ mod tests {
_ => panic!("unexpected menu item type"),
})
.collect();
// Settings and Pause Everything live in the main window (issue #84).
assert_eq!(labels, vec!["Open NextSync", "Log", "Quit"]);
// Settings and Pause Everything live in the main window (issue #84);
// About sits above Quit (issue #155).
assert_eq!(labels, vec!["Open NextSync", "Log", "Quit", "About"]);
reset_locale();
}

Expand All @@ -360,7 +381,7 @@ mod tests {
_ => panic!("unexpected menu item type"),
})
.collect();
assert_eq!(labels, vec!["Open NextSync", "Quit"]);
assert_eq!(labels, vec!["Open NextSync", "Quit", "About"]);
reset_locale();
}

Expand Down Expand Up @@ -395,7 +416,10 @@ mod tests {
_ => panic!("unexpected menu item type"),
})
.collect();
assert_eq!(labels, vec!["Abrir NextSync", "Registro", "Salir"]);
assert_eq!(
labels,
vec!["Abrir NextSync", "Registro", "Salir", "Acerca de"]
);
reset_locale();
}

Expand All @@ -412,6 +436,7 @@ mod tests {
assert_eq!(rx.try_recv().unwrap(), TrayAction::Open);
assert_eq!(rx.try_recv().unwrap(), TrayAction::Conflicts);
assert_eq!(rx.try_recv().unwrap(), TrayAction::Quit);
assert_eq!(rx.try_recv().unwrap(), TrayAction::About);
assert!(rx.try_recv().is_err(), "no extra actions should be sent");
}

Expand Down
Loading