Repository preferences: style fixes and use BUrl

Signed-off-by: Adrien Destugues <[email protected]>

Fixes #13194

src/preferences/repositories/AddRepoWindow.cpp

@@ -14,6 +14,7 @@
 #include <Catalog.h>
 #include <Clipboard.h>
 #include <LayoutBuilder.h>
+#include <Url.h>
 
 #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 "
+						B_TRANSLATE_COMMENT("This is not a valid URL.",
-							"protocol, for example http:// or https://",
 							"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("://");
+		BUrl testUrl(clipString.String());
-		if (ww == B_ERROR)
+		if (!testUrl.IsValid())
 			return B_ERROR;
 		else
 			fText->SetText(clipString);

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();
 };
 
 

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);
 };
 
 

src/preferences/repositories/RepositoriesView.cpp

@@ -18,6 +18,7 @@
 #include <MessageRunner.h>
 #include <ScrollBar.h>
 #include <SeparatorView.h>
+#include <Url.h>
 #include <package/PackageRoster.h>
 #include <package/RepositoryConfig.h>
 
@@ -224,7 +225,8 @@ RepositoriesView::MessageReceived(BMessage* message)
 {
 	switch (message->what)
 	{
-		case REMOVE_REPOS: {
+		case REMOVE_REPOS:
+		{
 			RepoRow* rowItem = dynamic_cast<RepoRow*>(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,29 +504,35 @@ RepositoriesView::_UpdateFromRepoConfig(RepoRow* rowItem)
 void
 RepositoriesView::AddManualRepository(BString url)
 {
+	BUrl newRepoUrl(url);
+	if (!newRepoUrl.IsValid())
+		return;
+	
 	BString name(kNewRepoDefaultName);
-	BString rootUrl = _GetRootUrl(url);
+	BString newPathIdentifier = _GetPathIdentifier(newRepoUrl.Path());
-	bool foundRoot = false;
+	bool foundMatchingRoot = false;
 	int32 index;
 	int32 listCount = fListView->CountRows();
 	for (index = 0; index < listCount; index++) {
 		RepoRow* repoItem = dynamic_cast<RepoRow*>(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
+		// Predict the repo name from another url with matching path root
-		if (foundRoot == false && rootUrl.ICompare(urlPtr,
+		if (!foundMatchingRoot) {
-				rootUrl.Length()) == 0) {
+			BString rowPathIdentifier = _GetPathIdentifier(rowRepoUrl.Path());
-			foundRoot = true;
+			 if (newPathIdentifier.ICompare(rowPathIdentifier) == 0) {
+				foundMatchingRoot = true;
 				name = repoItem->Name();
 			 }
 		}
+	}
 	RepoRow* newRepo = _AddRepo(name, url, false);
 	_FindSiblings();
 	fListView->DeselectAll();
@@ -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);
+	int32 index = urlPath.FindFirst("/");
-	if (rootEnd == B_ERROR)
+	if (index == B_ERROR)
-		return url;
+		return urlPath;
-	rootEnd = url.FindFirst("/", rootEnd + 1);
+	index = urlPath.FindFirst("/", index + 1);
-	if (rootEnd == B_ERROR)
+	if (index == B_ERROR)
-		return url;
+		return urlPath;
 	else
-		return url.Truncate(rootEnd);
+		return urlPath.Truncate(index);
 }
 
 
 status_t
 RepositoriesView::_EmptyList()
 {
-	BRow* row;
+	BRow* row = fListView->RowAt((int32)0, NULL);
-	while ((row = fListView->RowAt((int32)0, NULL)) != 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
+	// URL must be valid
-	if (url.FindFirst("://") == B_ERROR)
+	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<RepoRow*>(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) {
+	RepoRow* addedRow = new RepoRow(name, url, enabled);
-		addedRow = new RepoRow(name, url, enabled);
 	fListView->AddRow(addedRow);
-	}
 	return addedRow;
 }
 
@@ -698,17 +716,18 @@ RepositoriesView::_UpdateButtons()
 	RepoRow* rowItem = dynamic_cast<RepoRow*>(fListView->CurrentSelection());
 	// At least one row is selected
 	if (rowItem) {
-		bool someAreEnabled = false,
+		bool someAreEnabled = false;
-			someAreDisabled = false,
+		bool someAreDisabled = false;
-			someAreInQueue = false;
+		bool someAreInQueue = false;
 		int32 selectedCount = 0;
 		RepoRow* rowItem = dynamic_cast<RepoRow*>(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

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();
 };
 
 

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);
 	}

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();
 };
 
 

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

src/preferences/repositories/TaskLooper.h

@@ -54,11 +54,12 @@ public:
 	virtual void			MessageReceived(BMessage*);
 
 private:
-	BObjectList<Task>		fTaskQueue;
 	void					_RemoveAndDelete(Task* task);
 	static status_t			_DoTask(void* data);
 	static void				_AppendErrorDetails(BString& details,
 								JobStateListener* listener);
+	
+	BObjectList<Task>		fTaskQueue;
 	BMessenger				fReplyTarget;
 	BMessenger				fMessenger;
 };

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);
 }

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();
 };