From 856ecc7b19a4a9942cfe3010cab9bc3524870732 Mon Sep 17 00:00:00 2001 From: Brian Hill Date: Sun, 8 Jan 2017 19:40:01 -0500 Subject: [PATCH] Repository preferences: style fixes and use BUrl Signed-off-by: Adrien Destugues Fixes #13194 --- .../repositories/AddRepoWindow.cpp | 17 +-- src/preferences/repositories/AddRepoWindow.h | 3 +- .../repositories/RepositoriesSettings.h | 5 +- .../repositories/RepositoriesView.cpp | 115 ++++++++++-------- .../repositories/RepositoriesView.h | 42 +++---- .../repositories/RepositoriesWindow.cpp | 26 ++-- .../repositories/RepositoriesWindow.h | 5 +- src/preferences/repositories/TaskLooper.cpp | 21 +++- src/preferences/repositories/TaskLooper.h | 3 +- src/preferences/repositories/TaskTimer.cpp | 10 +- src/preferences/repositories/TaskTimer.h | 3 +- 11 files changed, 151 insertions(+), 99 deletions(-) diff --git a/src/preferences/repositories/AddRepoWindow.cpp b/src/preferences/repositories/AddRepoWindow.cpp index 1e0f325c6c..b163ce57b1 100644 --- a/src/preferences/repositories/AddRepoWindow.cpp +++ b/src/preferences/repositories/AddRepoWindow.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include "constants.h" @@ -76,14 +77,15 @@ AddRepoWindow::MessageReceived(BMessage* message) Quit(); break; - case ADD_BUTTON_PRESSED: { + case ADD_BUTTON_PRESSED: + { BString url(fText->Text()); if (url != "") { // URL must have a protocol - if (url.FindFirst("://") == B_ERROR) { + BUrl newRepoUrl(url); + if (!newRepoUrl.IsValid()) { BAlert* alert = new BAlert("error", - B_TRANSLATE_COMMENT("The URL must start with a " - "protocol, for example http:// or https://", + B_TRANSLATE_COMMENT("This is not a valid URL.", "Add URL error message"), kOKLabel, NULL, NULL, B_WIDTH_AS_USUAL, B_STOP_ALERT); alert->SetFeel(B_MODAL_APP_WINDOW_FEEL); @@ -100,6 +102,7 @@ AddRepoWindow::MessageReceived(BMessage* message) } break; } + default: BWindow::MessageReceived(message); } @@ -124,10 +127,10 @@ AddRepoWindow::_GetClipboardData() &stringLen); be_clipboard->Unlock(); - // The string must contain a web protocol + // The string must be a valid url BString clipString(string, stringLen); - int32 ww = clipString.FindFirst("://"); - if (ww == B_ERROR) + BUrl testUrl(clipString.String()); + if (!testUrl.IsValid()) return B_ERROR; else fText->SetText(clipString); diff --git a/src/preferences/repositories/AddRepoWindow.h b/src/preferences/repositories/AddRepoWindow.h index c6c17580ba..6c128f32c6 100644 --- a/src/preferences/repositories/AddRepoWindow.h +++ b/src/preferences/repositories/AddRepoWindow.h @@ -24,11 +24,12 @@ public: virtual void FrameResized(float newWidth, float newHeight); private: + status_t _GetClipboardData(); + BTextControl* fText; BButton* fAddButton; BButton* fCancelButton; BMessenger fReplyMessenger; - status_t _GetClipboardData(); }; diff --git a/src/preferences/repositories/RepositoriesSettings.h b/src/preferences/repositories/RepositoriesSettings.h index 8ceae1f5d7..cea7a46de2 100644 --- a/src/preferences/repositories/RepositoriesSettings.h +++ b/src/preferences/repositories/RepositoriesSettings.h @@ -29,11 +29,12 @@ public: BStringList& urlList); private: + BMessage _ReadFromFile(); + status_t _SaveToFile(BMessage settings); + BPath fFilePath; BFile fFile; status_t fInitStatus; - BMessage _ReadFromFile(); - status_t _SaveToFile(BMessage settings); }; diff --git a/src/preferences/repositories/RepositoriesView.cpp b/src/preferences/repositories/RepositoriesView.cpp index c1cd3cf502..4e7788ef2f 100644 --- a/src/preferences/repositories/RepositoriesView.cpp +++ b/src/preferences/repositories/RepositoriesView.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -224,7 +225,8 @@ RepositoriesView::MessageReceived(BMessage* message) { switch (message->what) { - case REMOVE_REPOS: { + case REMOVE_REPOS: + { RepoRow* rowItem = dynamic_cast(fListView->CurrentSelection()); if (!rowItem || !fRemoveButton->IsEnabled()) break; @@ -272,11 +274,13 @@ RepositoriesView::MessageReceived(BMessage* message) _SaveList(); break; } + case LIST_SELECTION_CHANGED: _UpdateButtons(); break; - case ITEM_INVOKED: { + case ITEM_INVOKED: + { // Simulates pressing whichever is the enabled button if (fEnableButton->IsEnabled()) { BMessage invokeMessage(ENABLE_BUTTON_PRESSED); @@ -287,7 +291,9 @@ RepositoriesView::MessageReceived(BMessage* message) } break; } - case ENABLE_BUTTON_PRESSED: { + + case ENABLE_BUTTON_PRESSED: + { BStringList names; bool paramsOK = true; // Check if there are multiple selections of the same repository, @@ -314,12 +320,14 @@ RepositoriesView::MessageReceived(BMessage* message) } break; } + case DISABLE_BUTTON_PRESSED: _AddSelectedRowsToQueue(); _UpdateButtons(); break; - case TASK_STARTED: { + case TASK_STARTED: + { int16 count; status_t result1 = message->FindInt16(key_count, &count); RepoRow* rowItem; @@ -328,7 +336,9 @@ RepositoriesView::MessageReceived(BMessage* message) _TaskStarted(rowItem, count); break; } - case TASK_COMPLETED_WITH_ERRORS: { + + case TASK_COMPLETED_WITH_ERRORS: + { BString errorDetails; status_t result = message->FindString(key_details, &errorDetails); if (result == B_OK) { @@ -350,7 +360,9 @@ RepositoriesView::MessageReceived(BMessage* message) _UpdateButtons(); break; } - case TASK_COMPLETED: { + + case TASK_COMPLETED: + { BString repoName = message->GetString(key_name, kNewRepoDefaultName.String()); int16 count; @@ -367,7 +379,9 @@ RepositoriesView::MessageReceived(BMessage* message) _UpdateButtons(); break; } - case TASK_CANCELED: { + + case TASK_CANCELED: + { int16 count; status_t result1 = message->FindInt16(key_count, &count); RepoRow* rowItem; @@ -380,18 +394,21 @@ RepositoriesView::MessageReceived(BMessage* message) _UpdateButtons(); break; } + case UPDATE_LIST: _RefreshList(); _UpdateButtons(); break; - case STATUS_VIEW_COMPLETED_TIMEOUT: { + case STATUS_VIEW_COMPLETED_TIMEOUT: + { int32 timerID; status_t result = message->FindInt32(key_ID, &timerID); if (result == B_OK && timerID == fLastCompletedTimerId) _UpdateStatusView(); break; } + default: BView::MessageReceived(message); } @@ -434,8 +451,9 @@ RepositoriesView::_TaskCompleted(RepoRow* rowItem, int16 count, BString& newName // Update row state and values rowItem->SetTaskState(STATE_NOT_IN_QUEUE); if (kNewRepoDefaultName.Compare(rowItem->Name()) == 0 - && newName.Compare("") != 0) + && newName.Compare("") != 0) { rowItem->SetName(newName.String()); + } _UpdateFromRepoConfig(rowItem); } @@ -486,27 +504,33 @@ RepositoriesView::_UpdateFromRepoConfig(RepoRow* rowItem) void RepositoriesView::AddManualRepository(BString url) { + BUrl newRepoUrl(url); + if (!newRepoUrl.IsValid()) + return; + BString name(kNewRepoDefaultName); - BString rootUrl = _GetRootUrl(url); - bool foundRoot = false; + BString newPathIdentifier = _GetPathIdentifier(newRepoUrl.Path()); + bool foundMatchingRoot = false; int32 index; int32 listCount = fListView->CountRows(); for (index = 0; index < listCount; index++) { RepoRow* repoItem = dynamic_cast(fListView->RowAt(index)); - const char* urlPtr = repoItem->Url(); + BUrl rowRepoUrl(repoItem->Url()); // Find an already existing URL - if (url.ICompare(urlPtr) == 0) { + if (newRepoUrl == rowRepoUrl) { (new BAlert("duplicate", B_TRANSLATE_COMMENT("This repository URL already exists.", "Error message"), kOKLabel))->Go(NULL); return; } - // Use the same name from another repo with the same root url - if (foundRoot == false && rootUrl.ICompare(urlPtr, - rootUrl.Length()) == 0) { - foundRoot = true; - name = repoItem->Name(); + // Predict the repo name from another url with matching path root + if (!foundMatchingRoot) { + BString rowPathIdentifier = _GetPathIdentifier(rowRepoUrl.Path()); + if (newPathIdentifier.ICompare(rowPathIdentifier) == 0) { + foundMatchingRoot = true; + name = repoItem->Name(); + } } } RepoRow* newRepo = _AddRepo(name, url, false); @@ -519,33 +543,28 @@ RepositoriesView::AddManualRepository(BString url) BString -RepositoriesView::_GetRootUrl(BString url) +RepositoriesView::_GetPathIdentifier(BString urlPath) { - // Find the protocol if it exists - int32 ww = url.FindFirst("://"); - if (ww == B_ERROR) - ww = 0; - else - ww += 3; // Find second / - int32 rootEnd = url.FindFirst("/", ww + 1); - if (rootEnd == B_ERROR) - return url; - rootEnd = url.FindFirst("/", rootEnd + 1); - if (rootEnd == B_ERROR) - return url; + int32 index = urlPath.FindFirst("/"); + if (index == B_ERROR) + return urlPath; + index = urlPath.FindFirst("/", index + 1); + if (index == B_ERROR) + return urlPath; else - return url.Truncate(rootEnd); + return urlPath.Truncate(index); } status_t RepositoriesView::_EmptyList() { - BRow* row; - while ((row = fListView->RowAt((int32)0, NULL)) != NULL) { + BRow* row = fListView->RowAt((int32)0, NULL); + while (row != NULL) { fListView->RemoveRow(row); delete row; + row = fListView->RowAt((int32)0, NULL); } return B_OK; } @@ -639,27 +658,26 @@ RepositoriesView::_SaveList() RepoRow* RepositoriesView::_AddRepo(BString name, BString url, bool enabled) { - // URL must have a protocol - if (url.FindFirst("://") == B_ERROR) + // URL must be valid + BUrl repoUrl(url); + if (!repoUrl.IsValid()) return NULL; - RepoRow* addedRow = NULL; int32 index; int32 listCount = fListView->CountRows(); // Find if the repo already exists in list for (index = 0; index < listCount; index++) { RepoRow* repoItem = dynamic_cast(fListView->RowAt(index)); - if (url.ICompare(repoItem->Url()) == 0) { + BUrl itemUrl(repoItem->Url()); + if (repoUrl == itemUrl) { // update name and enabled values - if (name.Compare(repoItem->Name()) != 0) + if (name != repoItem->Name()) repoItem->SetName(name.String()); repoItem->SetEnabled(enabled); - addedRow = repoItem; + return repoItem; } } - if (addedRow == NULL) { - addedRow = new RepoRow(name, url, enabled); - fListView->AddRow(addedRow); - } + RepoRow* addedRow = new RepoRow(name, url, enabled); + fListView->AddRow(addedRow); return addedRow; } @@ -698,17 +716,18 @@ RepositoriesView::_UpdateButtons() RepoRow* rowItem = dynamic_cast(fListView->CurrentSelection()); // At least one row is selected if (rowItem) { - bool someAreEnabled = false, - someAreDisabled = false, - someAreInQueue = false; + bool someAreEnabled = false; + bool someAreDisabled = false; + bool someAreInQueue = false; int32 selectedCount = 0; RepoRow* rowItem = dynamic_cast(fListView->CurrentSelection()); while (rowItem) { selectedCount++; uint32 taskState = rowItem->TaskState(); if ( taskState == STATE_IN_QUEUE_WAITING - || taskState == STATE_IN_QUEUE_RUNNING) + || taskState == STATE_IN_QUEUE_RUNNING) { someAreInQueue = true; + } if (rowItem->IsEnabled()) someAreEnabled = true; else diff --git a/src/preferences/repositories/RepositoriesView.h b/src/preferences/repositories/RepositoriesView.h index 875b72367a..2bf5465275 100644 --- a/src/preferences/repositories/RepositoriesView.h +++ b/src/preferences/repositories/RepositoriesView.h @@ -38,6 +38,27 @@ public: bool IsTaskRunning() { return fRunningTaskCount > 0; } private: + // Message helpers + void _AddSelectedRowsToQueue(); + void _TaskStarted(RepoRow* rowItem, int16 count); + void _TaskCompleted(RepoRow* rowItem, int16 count, + BString& newName); + void _TaskCanceled(RepoRow* rowItem, int16 count); + void _ShowCompletedStatusIfDone(); + void _UpdateFromRepoConfig(RepoRow* rowItem); + + // GUI functions + BString _GetPathIdentifier(BString urlPath); + status_t _EmptyList(); + void _InitList(); + void _RefreshList(); + void _UpdateListFromRoster(); + void _SaveList(); + RepoRow* _AddRepo(BString name, BString url, bool enabled); + void _FindSiblings(); + void _UpdateButtons(); + void _UpdateStatusView(); + RepositoriesSettings fSettings; RepositoriesListView* fListView; BView* fStatusContainerView; @@ -49,27 +70,6 @@ private: BButton* fRemoveButton; BButton* fEnableButton; BButton* fDisableButton; - - // Message helpers - void _AddSelectedRowsToQueue(); - void _TaskStarted(RepoRow* rowItem, int16 count); - void _TaskCompleted(RepoRow* rowItem, int16 count, - BString& newName); - void _TaskCanceled(RepoRow* rowItem, int16 count); - void _ShowCompletedStatusIfDone(); - void _UpdateFromRepoConfig(RepoRow* rowItem); - - // GUI functions - BString _GetRootUrl(BString url); - status_t _EmptyList(); - void _InitList(); - void _RefreshList(); - void _UpdateListFromRoster(); - void _SaveList(); - RepoRow* _AddRepo(BString name, BString url, bool enabled); - void _FindSiblings(); - void _UpdateButtons(); - void _UpdateStatusView(); }; diff --git a/src/preferences/repositories/RepositoriesWindow.cpp b/src/preferences/repositories/RepositoriesWindow.cpp index 8087419274..d2aad6c3b3 100644 --- a/src/preferences/repositories/RepositoriesWindow.cpp +++ b/src/preferences/repositories/RepositoriesWindow.cpp @@ -43,8 +43,9 @@ RepositoriesWindow::RepositoriesWindow() BScreen screen; BRect screenFrame = screen.Frame(); if (screenFrame.right < frame.right || screenFrame.left > frame.left - || screenFrame.top > frame.top || screenFrame.bottom < frame.bottom) + || screenFrame.top > frame.top || screenFrame.bottom < frame.bottom) { CenterOnScreen(); + } else MoveTo(frame.left, frame.top); Show(); @@ -126,29 +127,38 @@ RepositoriesWindow::MessageReceived(BMessage* message) { switch (message->what) { - case ADD_REPO_WINDOW: { + case ADD_REPO_WINDOW: + { BRect frame = Frame(); fAddWindow = new AddRepoWindow(frame, fMessenger); break; } - case ADD_REPO_URL: { + + case ADD_REPO_URL: + { BString url; status_t result = message->FindString(key_url, &url); if (result == B_OK) fView->AddManualRepository(url); break; } - case ADD_WINDOW_CLOSED: { + + case ADD_WINDOW_CLOSED: + { fAddWindow = NULL; break; } - case DELETE_KEY_PRESSED: { + + case DELETE_KEY_PRESSED: + { BMessage message(REMOVE_REPOS); fView->MessageReceived(&message); break; } + // captures pkgman changes while the Repositories application is running - case B_NODE_MONITOR: { + case B_NODE_MONITOR: + { // This preflet is making the changes, so ignore this message if (fView->IsTaskRunning()) break; @@ -159,7 +169,8 @@ RepositoriesWindow::MessageReceived(BMessage* message) { case B_ATTR_CHANGED: case B_ENTRY_CREATED: - case B_ENTRY_REMOVED: { + case B_ENTRY_REMOVED: + { PostMessage(UPDATE_LIST, fView); break; } @@ -167,6 +178,7 @@ RepositoriesWindow::MessageReceived(BMessage* message) } break; } + default: BWindow::MessageReceived(message); } diff --git a/src/preferences/repositories/RepositoriesWindow.h b/src/preferences/repositories/RepositoriesWindow.h index 3272bbff16..6570c42265 100644 --- a/src/preferences/repositories/RepositoriesWindow.h +++ b/src/preferences/repositories/RepositoriesWindow.h @@ -25,6 +25,9 @@ public: virtual void MessageReceived(BMessage*); private: + void _StartWatching(); + void _StopWatching(); + RepositoriesSettings fSettings; RepositoriesView* fView; AddRepoWindow* fAddWindow; @@ -34,8 +37,6 @@ private: status_t fPackageNodeStatus; bool fWatchingPackageNode; // true when package-repositories directory is being watched - void _StartWatching(); - void _StopWatching(); }; diff --git a/src/preferences/repositories/TaskLooper.cpp b/src/preferences/repositories/TaskLooper.cpp index 91f9bf1bac..ef9fb3d65b 100644 --- a/src/preferences/repositories/TaskLooper.cpp +++ b/src/preferences/repositories/TaskLooper.cpp @@ -115,7 +115,8 @@ TaskLooper::MessageReceived(BMessage* message) { switch (message->what) { - case DO_TASK: { + case DO_TASK: + { RepoRow* rowItem; status_t result = message->FindPointer(key_rowptr, (void**)&rowItem); if (result == B_OK) { @@ -170,9 +171,11 @@ TaskLooper::MessageReceived(BMessage* message) } break; } + case TASK_COMPLETED: case TASK_COMPLETED_WITH_ERRORS: - case TASK_CANCELED: { + case TASK_CANCELED: + { Task* task; status_t result = message->FindPointer(key_taskptr, (void**)&task); if (result == B_OK && fTaskQueue.HasItem(task)) { @@ -183,14 +186,17 @@ TaskLooper::MessageReceived(BMessage* message) if (message->what == TASK_COMPLETED_WITH_ERRORS) reply.AddString(key_details, task->resultErrorDetails); if (task->taskType == ENABLE_REPO - && task->name.Compare(task->resultName) != 0) + && task->name.Compare(task->resultName) != 0) { reply.AddString(key_name, task->resultName); + } fReplyTarget.SendMessage(&reply); _RemoveAndDelete(task); } break; } - case TASK_KILL_REQUEST: { + + case TASK_KILL_REQUEST: + { Task* task; status_t result = message->FindPointer(key_taskptr, (void**)&task); if (result == B_OK && fTaskQueue.HasItem(task)) { @@ -230,7 +236,8 @@ TaskLooper::_DoTask(void* data) JobStateListener listener; switch (task->taskType) { - case DISABLE_REPO: { + case DISABLE_REPO: + { BString nameParam(task->taskParam); BPackageKit::BContext context(decisionProvider, listener); BPackageKit::DropRepositoryRequest dropRequest(context, nameParam); @@ -249,7 +256,9 @@ TaskLooper::_DoTask(void* data) } break; } - case ENABLE_REPO: { + + case ENABLE_REPO: + { BString urlParam(task->taskParam); BPackageKit::BContext context(decisionProvider, listener); // Add repository diff --git a/src/preferences/repositories/TaskLooper.h b/src/preferences/repositories/TaskLooper.h index 1a671e6446..a79ab83882 100644 --- a/src/preferences/repositories/TaskLooper.h +++ b/src/preferences/repositories/TaskLooper.h @@ -54,11 +54,12 @@ public: virtual void MessageReceived(BMessage*); private: - BObjectList fTaskQueue; void _RemoveAndDelete(Task* task); static status_t _DoTask(void* data); static void _AppendErrorDetails(BString& details, JobStateListener* listener); + + BObjectList fTaskQueue; BMessenger fReplyTarget; BMessenger fMessenger; }; diff --git a/src/preferences/repositories/TaskTimer.cpp b/src/preferences/repositories/TaskTimer.cpp index 44b3a99962..f13a74876a 100644 --- a/src/preferences/repositories/TaskTimer.cpp +++ b/src/preferences/repositories/TaskTimer.cpp @@ -65,7 +65,8 @@ TaskTimer::MessageReceived(BMessage* message) { switch (message->what) { - case TASK_TIMEOUT: { + case TASK_TIMEOUT: + { fMessageRunner = NULL; if (fTimerIsRunning) { BString text(B_TRANSLATE_COMMENT("The task for repository" @@ -94,7 +95,9 @@ TaskTimer::MessageReceived(BMessage* message) } break; } - case TIMEOUT_ALERT_BUTTON_SELECTION: { + + case TIMEOUT_ALERT_BUTTON_SELECTION: + { fTimeoutAlert = NULL; // Timeout alert was invoked by user and timer still has not // been stopped @@ -126,9 +129,10 @@ TaskTimer::Start(const char* name) // Create a message runner that will send a TASK_TIMEOUT message if the // timer is not stopped - if (fMessageRunner == NULL) + if (fMessageRunner == NULL) { fMessageRunner = new BMessageRunner(fMessenger, &fTimeoutMessage, fTimeoutMicroSeconds, 1); + } else fMessageRunner->SetInterval(fTimeoutMicroSeconds); } diff --git a/src/preferences/repositories/TaskTimer.h b/src/preferences/repositories/TaskTimer.h index 00c0fec603..6da0873567 100644 --- a/src/preferences/repositories/TaskTimer.h +++ b/src/preferences/repositories/TaskTimer.h @@ -44,6 +44,8 @@ public: void Stop(const char* name); private: + int32 _NextAlertStackCount(); + int32 fTimeoutMicroSeconds; bool fTimerIsRunning; BString fRepositoryName; @@ -54,7 +56,6 @@ private: BAlert* fTimeoutAlert; BInvoker fTimeoutAlertInvoker; Task* fOwner; - int32 _NextAlertStackCount(); };