-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ability to switch between files in Git Diff widget #5965
Changes from 26 commits
2318d28
28ea7d4
5814596
9f7d30c
88bc819
a3e2c7c
e27895c
72fb977
347b2e0
638153b
9f18acd
3cd740e
f22b649
2f82bec
d148050
5d8744a
6f3ea37
241dca7
a1eac87
18ed447
5a6d745
e1bb8f4
c50272a
69715a6
96fed94
436dc59
286339c
e5208f5
d121a3d
cd6f922
89ad74e
65204e0
c4c0cdf
bea914d
21619ad
eaa74c5
2a1147e
7195d82
e2b5c84
0bd2dd9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
/******************************************************************************* | ||
* Copyright (c) 2012-2017 Red Hat, Inc. | ||
* All rights reserved. This program and the accompanying materials | ||
* are made available under the terms of the Eclipse Public License v1.0 | ||
* which accompanies this distribution, and is available at | ||
* http://www.eclipse.org/legal/epl-v10.html | ||
* | ||
* Contributors: | ||
* Red Hat, Inc. - initial API and implementation | ||
*******************************************************************************/ | ||
package org.eclipse.che.ide.ext.git.client.compare; | ||
|
||
import org.eclipse.che.ide.api.resources.Project; | ||
import org.eclipse.che.ide.ext.git.client.compare.FileStatus.Status; | ||
import org.eclipse.che.ide.util.loging.Log; | ||
|
||
import java.util.ArrayList; | ||
import java.util.LinkedHashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
|
||
import static com.google.common.base.Strings.isNullOrEmpty; | ||
import static org.eclipse.che.ide.ext.git.client.compare.FileStatus.defineStatus; | ||
|
||
/** | ||
* Describes changed in any way files in git comparison process. | ||
* | ||
* @author Mykola Morhun | ||
*/ | ||
public class AlteredFiles { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to see some tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, I think the current name is better because this class also parses raw diff. |
||
|
||
private final Project project; | ||
private final LinkedHashMap<String, Status> alteredFilesStatuses; | ||
private final List<String> alteredFilesList; | ||
|
||
/** | ||
* Parses raw git diff string and creates advanced representation. | ||
* | ||
* @param project | ||
* the project under diff operation | ||
* @param diff | ||
* plain result of git diff operation | ||
*/ | ||
public AlteredFiles(Project project, String diff) { | ||
this.project = project; | ||
|
||
alteredFilesStatuses = new LinkedHashMap<>(); | ||
if (!isNullOrEmpty(diff)) { | ||
for (String item : diff.split("\n")) { | ||
if (item.length() < 3 || item.charAt(1) != '\t') { | ||
throw new IllegalArgumentException("Invalid git diff format. Invalid record: " + item); | ||
} | ||
alteredFilesStatuses.put(item.substring(2, item.length()), defineStatus(item.substring(0, 1))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate the length of the item. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, but usually we obtain raw diff from git, so it should be valid. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trust no one! |
||
} | ||
} | ||
|
||
alteredFilesList = new ArrayList<>(alteredFilesStatuses.keySet()); | ||
} | ||
|
||
/** | ||
* Returns project in which git repository is located. | ||
*/ | ||
public Project getProject() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Javadoc? |
||
return project; | ||
} | ||
|
||
/** | ||
* Returns number of files in the diff. | ||
*/ | ||
public int getFilesQuantity() { | ||
return alteredFilesList.size(); | ||
} | ||
|
||
public boolean isEmpty() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Javadoc? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need javadoc for obviously methods? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not? if you added a javadoc for one method why don't you add it for others |
||
return 0 == alteredFilesList.size(); | ||
} | ||
|
||
/** | ||
* Returns this diff in map representation: altered file to its git status. | ||
*/ | ||
public Map<String, Status> getChangedFilesMap() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Javadoc? |
||
return alteredFilesStatuses; | ||
} | ||
|
||
/** | ||
* Returns list of altered files in this git diff. | ||
*/ | ||
public List<String> getAlteredFilesList() { | ||
return alteredFilesList; | ||
} | ||
|
||
public Status getStatusByFilePath(String relativePathToChangedFile) { | ||
return alteredFilesStatuses.get(relativePathToChangedFile); | ||
} | ||
|
||
public Status getStatusByIndex(int index) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Javadoc? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that it is already clear form the method name. |
||
return alteredFilesStatuses.get(alteredFilesList.get(index)); | ||
} | ||
|
||
public String getFileByIndex(int index) { | ||
return alteredFilesList.get(index); | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) return true; | ||
if (o == null || getClass() != o.getClass()) return false; | ||
AlteredFiles that = (AlteredFiles)o; | ||
return Objects.equals(project, that.project) && | ||
Objects.equals(alteredFilesStatuses, that.alteredFilesStatuses) && | ||
Objects.equals(alteredFilesList, that.alteredFilesList); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(project, alteredFilesStatuses, alteredFilesList); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please explain why we're changing a set for a list? We need to store same files several times?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we needn't, but git returns modified files in some order, so I just do not want to loose it. Also it is needed to navigate through this list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be
private AlteredFiles alteredFiles;
?