Skip to content

Commit 0624ca5

Browse files
authored
Refactor 4 (#80)
* fix: resolve 8 bugs across tray, settings, dialog, and lifecycle Bug fixes: - SystemTrayService.updateMenuState now rebuilds tray context menu after updating labels (menu was stale until next right-click) - BuiltinInstanceService.dispose() explicitly discards stopInstance() future instead of fire-and-forget, and removes duplicate _upnpService.shutdown() call (stopInstance() already handles UPnP shutdown) - AppearanceDialog: await async setThemeMode/setPrimaryColor calls so try-catch blocks can actually catch failures; add mounted guard before showing SnackBar after await - SettingsPage locale dialog: await setLocale() before Navigator.pop so save failures are not silently swallowed; add mounted guard - DebugProvider: replace in-place VNode list mutation (add/removeRange/clear) with immutable list reassignment to maintain VNode setter contract - Input: add didUpdateWidget to sync _obscureText when parent changes obscureText prop - App: remove duplicate loadSettings() call from _ThemeProviderState.initState (HomeWrapper._initialize already handles loading with proper await) * perf: reduce repeated settings reads and remove dead initialization code Performance: - Refactor _getConfiguredRpcPort/Secret to accept optional settings map parameter, eliminating 2 redundant _readSettingsSnapshot() calls in both _buildArgs() and getBuiltinInstanceConfig() (6 sync file reads → 4) - Remove unreachable duplicate guards in SystemTrayService.initialize() (dead code after in-flight await) * refactor: code quality improvements across settings, widgets, and app Code quality: - SettingsPage._buildSettingsGroup: remove no-op identity map (.map((child) => child)) - Settings._settingsFileName: change from final to static const - Settings: inline trivial _getDataDirectory() wrapper (single caller) - CustomAppBar/CardX: remove redundant key forwarding to inner widget (Flutter handles key reconciliation on the outermost widget) - App: extract _swapListener<T> helper to deduplicate 3 near-identical listener swap blocks in didChangeDependencies * chore: minor cleanup across settings, enums, and virtual window frame Cleanup: - Remove unused dart:async import in AutoHideWindowService - Fix _formatVersionLabel to display full version string (was showing only patch number, e.g. 'v3' instead of 'v1.2.3') - Remove redundant inline comments on enum values in enums.dart (comments restated the value names) - Simplify VirtualWindowFrame switch to a single conditional expression (first and third switch arms produced identical output) * Update packages * fix: resolve 5 bugs in RPC client, debug provider, and selection pruning Bug fixes: - DebugProvider: trim widgets.value alongside lines and _widgetCounts (widgets list was growing unboundedly while lines was capped at 100) - Aria2RpcClient._handleWebSocketMessage: add type guard for non-Map JSON responses (arrays/primitives caused NoSuchMethodError) - Aria2RpcClient._callHttpRpc: replace _httpClient! force-unwrap with null check + ConnectionFailedException (prevents crash after close()) - Aria2RpcClient: store WebSocket StreamSubscription in _webSocketSubscription field; cancel in close() and _connectWebSocket to prevent callbacks firing on disposed client - DownloadPage._pruneSelection: skip setState when no keys were removed (prevents unnecessary rebuild cycles on every data change) * perf: optimize filter, sort, and pieces grid rendering Performance: - DownloadPage._filterTasks: combine category filter, status/type filter, and search filter into a single retainWhere pass (was creating 3-5 intermediate lists via .where().toList() per filter call) - DownloadPage._filterTasks: inline sort key computation in comparator (eliminates O(n) sortKeys map allocation per sort) - TaskDetailsBtHelpers._buildPiecesGrid: replace Wrap+List.generate with CustomPaint painter (renders thousands of pieces without creating individual Container widgets, eliminates O(n) widget allocation) * refactor: code quality improvements across RPC client, dialogs, and models Code quality: - Aria2RpcClient._callHttpRpc: remove redundant response.body.contains check for 'Unauthorized' (structured JSON check is sufficient) - Aria2RpcClient.getVersion: delegate to getVersionInfo() to eliminate duplicate RPC call - Aria2RpcClient._handleWebSocketError: always wrap errors in ConnectionFailedException instead of leaking raw error types - AddTaskDialog: move outputField creation inside two-column branch (was allocated but unused in single-column path) - DownloadTask: add key getter ('::') to eliminate duplicate _taskKey function in download_page.dart and task_list_view.dart - FilterSelector: inline trivial _getInstanceFilterOptions wrapper * chore: minor cleanup in task service, utils, and helpers Cleanup: - DownloadTaskService: inline trivial _stoppingSeedingTip and _failedToStopSeedingMessage wrappers (single-line l10n accessors) - task_utils: use Uri.file() instead of Uri.parse('file://...') for correct platform-specific file URI construction (handles paths with spaces and special characters on Linux/macOS) - TaskDetailsBtHelpers: add explanatory comment to catch(_) block in parseTorrentMetadata (best-effort parsing returns empty on malformed torrent data) * fix: address review findings — guards, error handling, and formatting Bug fixes: - TaskDetailsBtHelpers._buildPiecesGrid: add empty pieces guard to prevent ArgumentError from .clamp(1, 0) when pieces list is empty - Aria2RpcClient._callHttpRpc: guard data['error'] is Map before accessing ['message'] to prevent TypeError on malformed responses - Aria2RpcClient._handleWebSocketMessage: same guard for data['error'] is Map before accessing ['message'] - AppearanceDialog: add try-catch to G and B slider onChangeEnd handlers (R slider already had error handling, G/B were missing) Formatting: - Run dart format on 7 files to comply with project 80-column style: virtual_window_frame, task_details_bt_helpers, download_page, enums, download_task_service, system_tray_service, aria2_rpc_client Skipped findings (not valid): - _connectWebSocket await subscription cancel: cancel() returns synchronously for non-pending operations, no race condition risk - settings_page setLocale try-catch: already has await + context.mounted guard, setLocale failures propagate as unhandled which is acceptable
1 parent b2d285b commit 0624ca5

23 files changed

Lines changed: 331 additions & 293 deletions

lib/app.dart

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,6 @@ class _ThemeProvider extends StatefulWidget {
4545
}
4646

4747
class _ThemeProviderState extends State<_ThemeProvider> {
48-
@override
49-
void initState() {
50-
super.initState();
51-
// Load settings
52-
Provider.of<Settings>(context, listen: false).loadSettings();
53-
}
54-
5548
@override
5649
Widget build(BuildContext context) {
5750
final display = context
@@ -279,38 +272,45 @@ class _MainWindowState extends State<MainWindow> with WindowListener, Loggable {
279272
@override
280273
void didChangeDependencies() {
281274
super.didChangeDependencies();
282-
final nextDownloadDataService = Provider.of<DownloadDataService>(
283-
context,
284-
listen: false,
275+
_downloadDataService = _swapListener(
276+
Provider.of<DownloadDataService>(context, listen: false),
277+
_downloadDataService,
278+
(l) => l?.removeListener(_handleDownloadNotifications),
279+
(l) => l?.addListener(_handleDownloadNotifications),
285280
);
286-
if (_downloadDataService != nextDownloadDataService) {
287-
_downloadDataService?.removeListener(_handleDownloadNotifications);
288-
_downloadDataService = nextDownloadDataService;
289-
_downloadDataService?.addListener(_handleDownloadNotifications);
290-
}
291281

292-
final nextInstanceManager = Provider.of<InstanceManager>(
293-
context,
294-
listen: false,
282+
_instanceManager = _swapListener(
283+
Provider.of<InstanceManager>(context, listen: false),
284+
_instanceManager,
285+
(l) => l?.removeListener(_handleInstanceManagerChanged),
286+
(l) => l?.addListener(_handleInstanceManagerChanged),
295287
);
296-
if (_instanceManager != nextInstanceManager) {
297-
_instanceManager?.removeListener(_handleInstanceManagerChanged);
298-
_instanceManager = nextInstanceManager;
299-
_instanceManager?.addListener(_handleInstanceManagerChanged);
300-
}
301288

302-
final nextSettings = Provider.of<Settings>(context, listen: false);
303-
if (_settings != nextSettings) {
304-
_settings?.removeListener(_handleSettingsChanged);
305-
_settings = nextSettings;
306-
_settings?.addListener(_handleSettingsChanged);
307-
}
289+
_settings = _swapListener(
290+
Provider.of<Settings>(context, listen: false),
291+
_settings,
292+
(l) => l?.removeListener(_handleSettingsChanged),
293+
(l) => l?.addListener(_handleSettingsChanged),
294+
);
308295

309296
WidgetsBinding.instance.addPostFrameCallback((_) {
310297
unawaited(_applyShellSettings());
311298
});
312299
}
313300

301+
T _swapListener<T>(
302+
T next,
303+
T? current,
304+
void Function(T? l) remove,
305+
void Function(T l) add,
306+
) {
307+
if (current != next) {
308+
remove(current);
309+
add(next);
310+
}
311+
return next;
312+
}
313+
314314
void _handleSettingsChanged() {
315315
unawaited(_handleTrayStateChanged());
316316
unawaited(_applyShellSettings());

lib/kit/provider/debug.dart

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ final class DebugProvider {
2828
lines.add('$title$level$message');
2929

3030
var widgetCount = 1;
31-
widgets.value.add(
31+
final newWidgets = <Widget>[
3232
Text.rich(
3333
TextSpan(
3434
children: [
@@ -47,10 +47,10 @@ final class DebugProvider {
4747
],
4848
),
4949
),
50-
);
50+
];
5151
if (record.stackTrace != null) {
5252
widgetCount++;
53-
widgets.value.add(
53+
newWidgets.add(
5454
SingleChildScrollView(
5555
scrollDirection: Axis.horizontal,
5656
child: Text(
@@ -61,24 +61,24 @@ final class DebugProvider {
6161
);
6262
}
6363
widgetCount++;
64-
widgets.value.add(UIs.height13);
64+
newWidgets.add(UIs.height13);
6565
_widgetCounts.add(widgetCount);
6666

6767
while (lines.length > maxLines) {
68-
final removed = _widgetCounts.removeAt(0);
68+
final removeCount = _widgetCounts.removeAt(0);
6969
lines.removeAt(0);
70-
if (widgets.value.length >= removed) {
71-
widgets.value.removeRange(0, removed);
70+
if (widgets.value.length >= removeCount) {
71+
widgets.value = widgets.value.sublist(removeCount);
7272
}
7373
}
74-
widgets.notify();
74+
75+
widgets.value = [...widgets.value, ...newWidgets];
7576
}
7677

7778
static void clear() {
78-
widgets.value.clear();
79+
widgets.value = [];
7980
lines.clear();
8081
_widgetCounts.clear();
81-
widgets.notify();
8282
}
8383

8484
static void copy() =>

lib/kit/widgets/appbar.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ class CustomAppBar extends StatelessWidget implements PreferredSizeWidget {
3030
@override
3131
Widget build(BuildContext context) {
3232
return AppBar(
33-
key: key,
3433
title: title,
3534
actions: actions,
3635
centerTitle: centerTitle,

lib/kit/widgets/card.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ class CardX extends StatelessWidget {
1919
@override
2020
Widget build(BuildContext context) {
2121
return Card(
22-
key: key,
2322
clipBehavior: clipBehavior,
2423
color: color,
2524
shape: RoundedRectangleBorder(borderRadius: radius ?? borderRadius),

lib/kit/widgets/input.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,14 @@ class Input extends StatefulWidget {
6969
class _InputState extends State<Input> {
7070
late bool _obscureText = widget.obscureText;
7171

72+
@override
73+
void didUpdateWidget(covariant Input oldWidget) {
74+
super.didUpdateWidget(oldWidget);
75+
if (widget.obscureText != oldWidget.obscureText) {
76+
_obscureText = widget.obscureText;
77+
}
78+
}
79+
7280
@override
7381
Widget build(BuildContext context) {
7482
final icon = widget.icon != null

lib/kit/widgets/virtual_window_frame.dart

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,17 @@ class VirtualWindowFrame extends StatelessWidget {
2222

2323
@override
2424
Widget build(BuildContext context) {
25-
final content = switch (CustomAppBar.sysStatusBarHeight) {
26-
0.0 => child,
27-
_ when showCaption && WindowFrameConfig.showCaption => Column(
28-
children: [
29-
_WindowCaption(title: title),
30-
Expanded(child: child),
31-
],
32-
),
33-
_ => child,
34-
};
25+
final content =
26+
(CustomAppBar.sysStatusBarHeight != 0.0 &&
27+
showCaption &&
28+
WindowFrameConfig.showCaption)
29+
? Column(
30+
children: [
31+
_WindowCaption(title: title),
32+
Expanded(child: child),
33+
],
34+
)
35+
: child;
3536
return wm.VirtualWindowFrame(child: content);
3637
}
3738
}

lib/models/settings.dart

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,16 +84,11 @@ class Settings extends ChangeNotifier with Loggable {
8484
'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36'; // User agent
8585

8686
// Settings file name
87-
final String _settingsFileName = 'settings.json';
87+
static const String _settingsFileName = 'settings.json';
8888

8989
// Constructor initialization
9090
Settings();
9191

92-
/// Get program data directory
93-
Directory _getDataDirectory() {
94-
return getAppDataDirectory();
95-
}
96-
9792
Future<String> _defaultDownloadDirectory() {
9893
return Future.value(getDefaultDownloadDirectorySync());
9994
}
@@ -165,7 +160,7 @@ class Settings extends ChangeNotifier with Loggable {
165160

166161
/// Get settings file path
167162
String _getSettingsFilePath() {
168-
final dataDir = _getDataDirectory();
163+
final dataDir = getAppDataDirectory();
169164
final configDir = Directory(p.join(dataDir.path, 'config'));
170165
if (!configDir.existsSync()) {
171166
configDir.createSync(recursive: true);

lib/pages/download_page/components/add_task_dialog.dart

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -559,18 +559,6 @@ class _AddTaskDialogState extends State<AddTaskDialog>
559559
return LayoutBuilder(
560560
builder: (context, constraints) {
561561
final useTwoColumns = constraints.maxWidth >= 480;
562-
final splitField = _buildSplitStepper(l10n);
563-
final outputField = Expanded(
564-
flex: 3,
565-
child: TextField(
566-
controller: outputFileNameController,
567-
enabled: !_isSubmitting,
568-
decoration: InputDecoration(
569-
labelText: l10n.renameOutput,
570-
hintText: l10n.renameOutputPlaceholder,
571-
),
572-
),
573-
);
574562

575563
if (!useTwoColumns) {
576564
return Column(
@@ -589,6 +577,19 @@ class _AddTaskDialogState extends State<AddTaskDialog>
589577
);
590578
}
591579

580+
final splitField = _buildSplitStepper(l10n);
581+
final outputField = Expanded(
582+
flex: 3,
583+
child: TextField(
584+
controller: outputFileNameController,
585+
enabled: !_isSubmitting,
586+
decoration: InputDecoration(
587+
labelText: l10n.renameOutput,
588+
hintText: l10n.renameOutputPlaceholder,
589+
),
590+
),
591+
);
592+
592593
return Row(
593594
crossAxisAlignment: CrossAxisAlignment.start,
594595
children: [outputField, const SizedBox(width: 12), splitField],

lib/pages/download_page/components/filter_selector.dart

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ class FilterSelector extends StatelessWidget {
8888
borderRadius: BorderRadius.circular(20),
8989
),
9090
),
91-
..._getInstanceFilterOptions().map((instanceId) {
91+
...instanceIds.map((instanceId) {
9292
final isSelected = selectedInstanceId == instanceId;
9393
final instanceColor = colorScheme.tertiary;
9494
final instanceName =
@@ -220,10 +220,6 @@ class FilterSelector extends StatelessWidget {
220220
}
221221
}
222222

223-
List<String> _getInstanceFilterOptions() {
224-
return instanceIds;
225-
}
226-
227223
List<FilterOption> _getFilterOptionsForCurrentCategory() {
228224
switch (currentCategoryType) {
229225
case CategoryType.byStatus:

0 commit comments

Comments
 (0)