Refactor GitDiff::parseDiff to return a Git::Diff object instead …

…of using an output parameter

src/FileDiffWidget.cpp

@@ -667,7 +667,7 @@ void FileDiffWidget::updateDiffView(Git::Diff const &info, bool uncommited)
 		Git::Diff diff;
 		if (isValidID_(info.blob.a_id_or_path) && isValidID_(info.blob.b_id_or_path)) {
 			std::string text = diffObjects(info.blob.a_id_or_path, info.blob.b_id_or_path).toStdString();
-			GitDiff::parseDiff(text, &info, &diff);
+			diff = GitDiff::parseDiff(text, &info);
 		} else {
 			diff = info;
 		}
@@ -698,13 +698,8 @@ void FileDiffWidget::updateDiffView_(QString const &id_left, QString const &id_r
 	GitRunner g = git();
 	if (!g.isValidWorkingCopy()) return;
 
-	Git::Diff diff;
-	diff.path = path;
-	diff.blob.a_id_or_path = id_left;
-	diff.blob.b_id_or_path = id_right;
-	diff.mode = "0";
-	std::string text = diffObjects(diff.blob.a_id_or_path, diff.blob.b_id_or_path).toStdString();
-	GitDiff::parseDiff(text, &diff, &diff);
+	std::string text = diffObjects(id_left, id_right).toStdString();
+	Git::Diff diff = GitDiff::parseDiff(text, &diff);
 
 	Git::Object obj = catFile(g, diff.blob.a_id_or_path);
 	setSideBySide(diff, obj.content, false, g.workingDir());

src/FileHistoryWindow.cpp

@@ -132,31 +132,6 @@ void FileHistoryWindow::collectFileHistory()
 	ui->tableWidget_log->setCurrentCell(0, 0);
 }
 
-class FindFileIdThread : public QThread {
-private:
-	MainWindow *mainwindow()
-	{
-		return global->mainwindow;
-	}
-	GitRunner g;
-	QString commit_id;
-	QString file;
-public:
-	QString result;
-	FindFileIdThread(GitRunner g, QString const &commit_id, QString const &file)
-	{
-		this->g = g;
-		this->commit_id = commit_id;
-		this->file = file;
-	}
-
-protected:
-	void run() override
-	{
-		result = mainwindow()->findFileID(commit_id, file);
-	}
-};
-
 void FileHistoryWindow::updateDiffView()
 {
 	Q_ASSERT(m->g.isValidWorkingCopy());
@@ -168,19 +143,16 @@ void FileHistoryWindow::updateDiffView()
 		Git::CommitItem const &commit_left = m->commit_item_list[row + 1]; // older
 		Git::CommitItem const &commit_right = m->commit_item_list[row];    // newer
 
-		FindFileIdThread left_thread(m->g.dup(), commit_left.commit_id.toQString(), m->path);
-		FindFileIdThread right_thread(m->g.dup(), commit_right.commit_id.toQString(), m->path);
-		left_thread.start();
-		right_thread.start();
-		left_thread.wait();
-		right_thread.wait();
-		QString id_left = left_thread.result;
-		QString id_right = right_thread.result;
+		auto FindFileID = [&](Git::CommitItem const &commit){
+			return mainwindow()->findFileID(commit.commit_id, m->path);
+		};
+		QString id_left = FindFileID(commit_left);
+		QString id_right = FindFileID(commit_right);
 
 		ui->widget_diff_view->updateDiffView_(id_left, id_right, m->path);
 	} else if (row >= 0 && row < (int)m->commit_item_list.size()) {
 		Git::CommitItem const &commit = m->commit_item_list[row];    // newer
-		QString id = mainwindow()->findFileID(commit.commit_id.toQString(), m->path);
+		QString id = mainwindow()->findFileID(commit.commit_id, m->path);
 
 		Git::Diff diff(id, m->path, {});
 		ui->widget_diff_view->updateDiffView(diff, false);
@@ -192,23 +164,6 @@ void FileHistoryWindow::on_tableWidget_log_currentItemChanged(QTableWidgetItem *
 	updateDiffView();
 }
 
-//void FileHistoryWindow::onMoveNextItem()
-//{
-//	int row = ui->tableWidget_log->currentRow();
-//	int count = ui->tableWidget_log->rowCount();
-//	if (row + 1 < count) {
-//		ui->tableWidget_log->setCurrentCell(row + 1, 0, QItemSelectionModel::ClearAndSelect);
-//	}
-//}
-
-//void FileHistoryWindow::onMovePreviousItem()
-//{
-//	int row = ui->tableWidget_log->currentRow();
-//	if (row > 0) {
-//		ui->tableWidget_log->setCurrentCell(row - 1, 0, QItemSelectionModel::ClearAndSelect);
-//	}
-//}
-
 void FileHistoryWindow::on_tableWidget_log_customContextMenuRequested(const QPoint &pos)
 {
 	(void)pos;

src/FileHistoryWindow.h

@@ -31,9 +31,7 @@ class FileHistoryWindow : public QDialog {
 	void prepare(GitRunner g, QString const &path);
 private slots:
 	void on_tableWidget_log_currentItemChanged(QTableWidgetItem *current, QTableWidgetItem *previous);
-
 	void on_tableWidget_log_customContextMenuRequested(const QPoint &pos);
-
 };
 
 #endif // FILEHISTORYWINDOW_H

src/GitDiff.cpp

@@ -78,27 +78,30 @@ QString GitDiff::diffFiles(GitRunner g, QString const &a_path, QString const &b_
 	return g.diff_file(a_path, b_path);
 }
 
-void GitDiff::parseDiff(std::string const &s, Git::Diff const *info, Git::Diff *out)
+Git::Diff GitDiff::parseDiff(std::string const &s, Git::Diff const *info)
 {
+	Git::Diff out;
+
 	std::vector<std::string_view> lines = misc::splitLinesV(s, false);
 
-	out->diff = QString("diff --git ") + ("a/" + info->path) + ' ' + ("b/" + info->path);
-	out->index = QString("index ") + info->blob.a_id_or_path + ".." + info->blob.b_id_or_path + ' ' + info->mode;
-	out->path = info->path;
-	out->blob = info->blob;
-	out->a_submodule.item = info->a_submodule.item;
-	out->a_submodule.commit = info->a_submodule.commit;
-	out->b_submodule.item = info->b_submodule.item;
-	out->b_submodule.commit = info->b_submodule.commit;
+	out.diff = QString("diff --git ") + ("a/" + info->path) + ' ' + ("b/" + info->path);
+	out.index = QString("index ") + info->blob.a_id_or_path + ".." + info->blob.b_id_or_path + ' ' + info->mode;
+	out.path = info->path;
+	out.blob = info->blob;
+	out.a_submodule.item = info->a_submodule.item;
+	out.a_submodule.commit = info->a_submodule.commit;
+	out.b_submodule.item = info->b_submodule.item;
+	out.b_submodule.commit = info->b_submodule.commit;
+	out.mode = "0";
 
 	bool atat = false;
 	for (std::string_view const &s : lines) {
 		std::string line = std::string{s};
 		int c = line[0] & 0xff;
 		if (c == '@') {
 			if (strncmp(line.c_str(), "@@ ", 3) == 0) {
-				out->hunks.push_back(Git::Hunk());
-				out->hunks.back().at = line;
+				out.hunks.push_back(Git::Hunk());
+				out.hunks.back().at = line;
 				atat = true;
 			}
 		} else {
@@ -113,13 +116,15 @@ void GitDiff::parseDiff(std::string const &s, Git::Diff const *info, Git::Diff *
 					}
 				}
 				if (atat) {
-					if (!out->hunks.isEmpty()) {
-						out->hunks.back().lines.push_back(line);
+					if (!out.hunks.isEmpty()) {
+						out.hunks.back().lines.push_back(line);
 					}
 				}
 			}
 		}
 	}
+
+	return out;
 }
 
 void GitDiff::retrieveCompleteTree(GitRunner g, QString const &dir, GitTreeItemList const *files, QList<Git::Diff> *diffs)
@@ -222,7 +227,7 @@ QList<Git::Diff> GitDiff::diff(GitRunner g, Git::Hash const &id, const QList<Git
 		}
 	} else { // 無効なIDなら、HEADと作業コピーのdiff
 
-		QString head_id = objcache_->revParse(g, "HEAD").toQString();
+		Git::Hash head_id = objcache_->revParse(g, "HEAD");
 		Git::FileStatusList stats = g.status_s(); // git status
 
 		GitCommitTree head_tree(objcache_);
@@ -388,10 +393,10 @@ void GitCommitTree::parseTree(GitRunner g, QString const &tree_id)
 	parseGitTreeObject(g, objcache, tree_id, QString(), &root_item_list);
 }
 
-QString GitCommitTree::parseCommit(GitRunner g, QString const &commit_id) // TODO: change commit_id as Git::Hash
+QString GitCommitTree::parseCommit(GitRunner g, Git::Hash const &commit_id)
 {
 	GitCommit commit;
-	GitCommit::parseCommit(g, objcache, Git::Hash(commit_id), &commit);
+	GitCommit::parseCommit(g, objcache, commit_id, &commit);
 	parseTree(g, commit.tree_id);
 	return commit.tree_id;
 }
@@ -405,10 +410,9 @@ QString GitCommitTree::parseCommit(GitRunner g, QString const &commit_id) // TOD
  * @param file
  * @return
  */
-QString lookupFileID(GitRunner g, GitObjectCache *objcache, QString const &commit_id, QString const &file)
+QString lookupFileID(GitRunner g, GitObjectCache *objcache, Git::Hash const &commit_id, QString const &file)
 {
 	GitCommitTree commit_tree(objcache);
 	commit_tree.parseCommit(g, commit_id);
-	QString id = commit_tree.lookup(g, file);
-	return id;
+	return commit_tree.lookup(g, file);
 }

src/GitDiff.h

@@ -33,12 +33,12 @@ class GitDiff {
 public:
 	static QString diffObjects(GitRunner g, QString const &a_id, QString const &b_id);
 	static QString diffFiles(GitRunner g, QString const &a_path, QString const &b_path);
-	static void parseDiff(std::string const &s, const Git::Diff *info, Git::Diff *out);
+	static Git::Diff parseDiff(std::string const &s, const Git::Diff *info);
 	static QString makeKey(const QString &a_id, const QString &b_id);
 	static QString makeKey(const Git::Diff &diff);
 	static QString prependPathPrefix(QString const &path);
 };
 
-QString lookupFileID(GitRunner g, GitObjectCache *objcache, QString const &commit_id, QString const &file);
+QString lookupFileID(GitRunner g, GitObjectCache *objcache, const Git::Hash &commit_id, QString const &file);
 
 #endif // GITDIFF_H

src/GitObjectManager.cpp

@@ -242,8 +242,8 @@ size_t GitObjectCache::size() const
 void GitObjectCache::clear()
 {
 	items_.clear();
-	revparsemap.clear();
-	object_manager.setup();
+	rev_parse_map_.clear();
+	object_manager_.setup();
 }
 
 Git::Hash GitObjectCache::revParse(GitRunner g, QString const &name)
@@ -252,8 +252,8 @@ Git::Hash GitObjectCache::revParse(GitRunner g, QString const &name)
 
 	{
 		// QMutexLocker lock(&object_manager.mutex);
-		auto it = revparsemap.find(name);
-		if (it != revparsemap.end()) {
+		auto it = rev_parse_map_.find(name);
+		if (it != rev_parse_map_.end()) {
 			return it->second;
 		}
 	}
@@ -262,7 +262,7 @@ Git::Hash GitObjectCache::revParse(GitRunner g, QString const &name)
 
 	{
 		// QMutexLocker lock(&object_manager.mutex);
-		revparsemap[name] = id;
+		rev_parse_map_[name] = id;
 		return id;
 	}
 }
@@ -276,20 +276,17 @@ Git::Object GitObjectCache::catFile(GitRunner g, Git::Hash const &id)
 		size_t i = n;
 		while (i > 0) {
 			i--;
-			if (items_[i]) {
-				if (items_[i]->id == id) {
-					ItemPtr item = items_[i];
-					if (i + 1 < n) { // move to last
-						items_.erase(items_.begin() + i);
-						items_.push_back(item);
-					}
-					Git::Object obj;
-					obj.type = item->type;
-					obj.content = item->ba;
-					return obj;
+			Q_ASSERT(items_[i]);
+			if (items_[i]->id == id) {
+				ItemPtr item = items_[i];
+				if (i + 1 < n) { // move to last
+					items_.erase(items_.begin() + i);
+					items_.push_back(item);
 				}
-			} else {
-				items_.erase(items_.begin() + i);
+				Git::Object obj;
+				obj.type = item->type;
+				obj.content = item->ba;
+				return obj;
 			}
 		}
 
@@ -315,7 +312,7 @@ Git::Object GitObjectCache::catFile(GitRunner g, Git::Hash const &id)
 		return obj;
 	};
 
-	if (object_manager.catFile(g, id, &ba, &type)) { // 独自実装のファイル取得
+	if (object_manager_.catFile(g, id, &ba, &type)) { // 独自実装のファイル取得
 		return Store();
 	}
 

src/GitObjectManager.h

@@ -45,10 +45,10 @@ class GitObjectCache {
 		Git::Object::Type type;
 	};
 private:
-	GitObjectManager object_manager;
+	GitObjectManager object_manager_;
 	using ItemPtr = std::shared_ptr<Item>;
 	std::vector<ItemPtr> items_;
-	std::map<QString, Git::Hash> revparsemap;
+	std::map<QString, Git::Hash> rev_parse_map_;
 	size_t size() const;
 public:
 	void clear();
@@ -110,7 +110,7 @@ class GitCommitTree {
 	bool lookup(GitRunner g, QString const &file, GitTreeItem *out);
 
 	void parseTree(GitRunner g, QString const &tree_id);
-	QString parseCommit(GitRunner g, QString const &commit_id);
+	QString parseCommit(GitRunner g, const Git::Hash &commit_id);
 
 	GitTreeItemList const *treelist() const
 	{

src/MainWindow.cpp

@@ -5685,7 +5685,7 @@ void MainWindow::emitWriteLog(const LogData &logdata)
 	emit sigWriteLog(logdata);
 }
 
-QString MainWindow::findFileID(const QString &commit_id, const QString &file)
+QString MainWindow::findFileID(Git::Hash const &commit_id, const QString &file)
 {
 	return lookupFileID(git(), getObjCache(), commit_id, file);
 }

src/MainWindow.h

@@ -506,7 +506,7 @@ class MainWindow : public QMainWindow {
 	TextEditorThemePtr themeForTextEditor();
 	static bool isValidWorkingCopy(GitRunner g);
 	void emitWriteLog(LogData const &logdata);
-	QString findFileID(const QString &commit_id, const QString &file);
+	QString findFileID(const Git::Hash &commit_id, const QString &file);
 	const Git::CommitItem &commitItem(int row) const;
 	const Git::CommitItem &commitItem(Git::Hash const &id) const;
 	QImage committerIcon(int row, QSize size) const;