Skip to content

Replacing StringUtil.escapeXml with more optimized XmlStringUtil.escapeText. Lazy logs #31

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.

Sign up for GitHub

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

Merged
VISTALL merged 1 commit into from
Mar 4, 2026
Merged

Replacing StringUtil.escapeXml with more optimized XmlStringUtil.escapeText. Lazy logs #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 43 additions & 43 deletions plugin/src/main/java/git4idea/GitUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import consulo.application.progress.Task;
import consulo.git.localize.GitLocalize;
import consulo.localize.LocalizeValue;
import consulo.logging.Logger;
import consulo.project.Project;
import consulo.ui.annotation.RequiredUIAccess;
import consulo.ui.ex.awt.DialogBuilder;
Expand Down Expand Up @@ -59,6 +58,8 @@
import git4idea.util.StringScanner;
import jakarta.annotation.Nonnull;
import jakarta.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -98,7 +99,7 @@ private GitRepositoryNotFoundException(@Nonnull FilePath filePath) {
public static final String MERGE_HEAD = "MERGE_HEAD";

private static final String SUBMODULE_REPO_PATH_PREFIX = "gitdir:";
private final static Logger LOG = Logger.getInstance(GitUtil.class);
private final static Logger LOG = LoggerFactory.getLogger(GitUtil.class);
private static final String HEAD_FILE = "HEAD";

private static final Pattern HASH_STRING_PATTERN = Pattern.compile("[a-fA-F0-9]{40}");
Expand Down Expand Up @@ -193,7 +194,7 @@ private static String readContent(@Nonnull VirtualFile dotGit) {
content = readFile(dotGit);
}
catch (IOException e) {
LOG.error("Couldn't read the content of " + dotGit, e);
LOG.error("Couldn't read the content of {}", dotGit, e);
return null;
}
return content;
Expand All @@ -210,7 +211,7 @@ public static String readFile(@Nonnull VirtualFile file) throws IOException {
return new String(file.contentsToByteArray());
}
catch (IOException e) {
LOG.info(String.format("IOException while reading %s (attempt #%s)", file, attempt));
LOG.info("IOException while reading {} (attempt #{})", file, attempt);
if (attempt >= ATTEMPTS - 1) {
throw e;
}
Expand Down Expand Up @@ -337,7 +338,7 @@ public static Date parseTimestampWithNFEReport(String value, GitHandler handler,
return parseTimestamp(value);
}
catch (NumberFormatException e) {
LOG.error("annotate(). NFE. Handler: " + handler + ". Output: " + gitOutput, e);
LOG.error("annotate(). NFE. Handler: {}. Output: {}", handler, gitOutput, e);
return new Date();
}
}
Expand Down Expand Up @@ -565,7 +566,7 @@ public static void getLocalCommittedChanges(
parametersSpecifier.accept(h);

String output = h.run();
LOG.debug("getLocalCommittedChanges output: '" + output + "'");
LOG.debug("getLocalCommittedChanges output: '{}'", output);
StringScanner s = new StringScanner(output);
StringBuilder sb = new StringBuilder();
boolean firstStep = true;
Expand Down Expand Up @@ -797,7 +798,7 @@ public static Collection<GitRepository> getRepositoriesFromRoots(
for (VirtualFile root : roots) {
GitRepository repo = repositoryManager.getRepositoryForRoot(root);
if (repo == null) {
LOG.error("Repository not found for root " + root);
LOG.error("Repository not found for root {}", root);
}
else {
repositories.add(repo);
Expand All @@ -823,7 +824,7 @@ public static Collection<String> getPathsDiffBetweenRefs(
String range = beforeRef + ".." + afterRef;
GitCommandResult result = git.diff(repository, parameters, range);
if (!result.success()) {
LOG.info(String.format("Couldn't get diff in range [%s] for repository [%s]", range, repository.toLogString()));
LOG.info("Couldn't get diff in range [{}] for repository [{}]", range, repository.toLogString());
return Collections.emptyList();
}

Expand All @@ -849,7 +850,7 @@ public static GitRepository getRepositoryForRootOrLogError(@Nonnull Project proj
GitRepositoryManager manager = getRepositoryManager(project);
GitRepository repository = manager.getRepositoryForRoot(root);
if (repository == null) {
LOG.error("Repository is null for root " + root);
LOG.error("Repository is null for root {}", root);
}
return repository;
}
Expand Down Expand Up @@ -957,7 +958,7 @@ public static VirtualFile findRefreshFileOrLog(@Nonnull String absolutePath) {
file = LocalFileSystem.getInstance().refreshAndFindFileByPath(absolutePath);
}
if (file == null) {
LOG.warn("VirtualFile not found for " + absolutePath);
LOG.warn("VirtualFile not found for {}", absolutePath);
}
return file;
}
Expand Down Expand Up @@ -990,21 +991,18 @@ public static List<Change> findLocalChangesForPaths(
for (String path : affectedPaths) {
String absolutePath = relativePaths ? toAbsolute(root, path) : path;
VirtualFile file = findRefreshFileOrLog(absolutePath);
if (file != null) {
Change change = changeListManager.getChange(file);
if (change != null) {
affectedChanges.add(change);
}
else {
String message = "Change is not found for " + file.getPath();
if (changeListManager.isInUpdate()) {
message += " because ChangeListManager is being updated.";
LOG.debug(message);
}
else {
LOG.info(message);
}
}
if (file == null) {
continue;
}
Change change = changeListManager.getChange(file);
if (change != null) {
affectedChanges.add(change);
}
else if (changeListManager.isInUpdate()) {
LOG.debug("Change is not found for {} because ChangeListManager is being updated.", file.getPath());
}
else {
LOG.info("Change is not found for {}", file.getPath());
}
}
return affectedChanges;
Expand Down Expand Up @@ -1102,30 +1100,32 @@ public static Collection<GitRepository> getRepositoriesInState(@Nonnull Project
public static boolean isCaseOnlyChange(@Nonnull String oldPath, @Nonnull String newPath) {
if (oldPath.equalsIgnoreCase(newPath)) {
if (oldPath.equals(newPath)) {
LOG.error("Comparing perfectly equal paths: " + newPath);
LOG.error("Comparing perfectly equal paths: {}", newPath);
}
return true;
}
return false;
}

@Nonnull
public static String getLogString(@Nonnull String root, @Nonnull Collection<Change> changes) {
return StringUtil.join(
changes,
change -> {
ContentRevision after = change.getAfterRevision();
ContentRevision before = change.getBeforeRevision();
return switch (change.getType()) {
case NEW -> "A: " + getRelativePath(root, assertNotNull(after));
case DELETED -> "D: " + getRelativePath(root, assertNotNull(before));
case MOVED ->
"M: " + getRelativePath(root, assertNotNull(before)) + " -> " + getRelativePath(root, assertNotNull(after));
default -> "M: " + getRelativePath(root, assertNotNull(after));
};
},
", "
);
public static record LogString(@Nonnull String root, @Nonnull Collection<Change> changes) {
@Override
public String toString() {
return StringUtil.join(
changes,
change -> {
ContentRevision after = change.getAfterRevision();
ContentRevision before = change.getBeforeRevision();
return switch (change.getType()) {
case NEW -> "A: " + getRelativePath(root, assertNotNull(after));
case DELETED -> "D: " + getRelativePath(root, assertNotNull(before));
case MOVED ->
"M: " + getRelativePath(root, assertNotNull(before)) + " -> " + getRelativePath(root, assertNotNull(after));
default -> "M: " + getRelativePath(root, assertNotNull(after));
};
},
", "
);
}
}

@Nullable
Expand Down
4 changes: 2 additions & 2 deletions plugin/src/main/java/git4idea/actions/GitInit.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import consulo.ui.ex.action.DumbAwareAction;
import consulo.ui.ex.awt.Messages;
import consulo.ui.ex.awt.UIUtil;
import consulo.util.lang.StringUtil;
import consulo.util.lang.xml.XmlStringUtil;
import consulo.versionControlSystem.ProjectLevelVcsManager;
import consulo.versionControlSystem.VcsNotifier;
import consulo.versionControlSystem.change.VcsDirtyScopeManager;
Expand Down Expand Up @@ -85,7 +85,7 @@ private static void doInit(final Project project, FileChooserDescriptor fcd, Vir
//noinspection RequiredXAction
if (GitUtil.isUnderGit(root) && Messages.showYesNoDialog(
project,
GitLocalize.initWarningAlreadyUnderGit(StringUtil.escapeXml(root.getPresentableUrl())).get(),
GitLocalize.initWarningAlreadyUnderGit(XmlStringUtil.escapeText(root.getPresentableUrl())).get(),
GitLocalize.initWarningTitle().get(),
UIUtil.getWarningIcon()
) != Messages.YES) {
Expand Down
34 changes: 17 additions & 17 deletions plugin/src/main/java/git4idea/checkin/GitCheckinEnvironment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import consulo.ide.ServiceManager;
import consulo.language.editor.ui.awt.*;
import consulo.localize.LocalizeValue;
import consulo.logging.Logger;
import consulo.platform.Platform;
import consulo.platform.base.localize.CommonLocalize;
import consulo.project.Project;
Expand All @@ -39,6 +38,7 @@
import consulo.util.lang.function.Functions;
import consulo.util.lang.function.PairConsumer;
import consulo.util.lang.ref.SimpleReference;
import consulo.util.lang.xml.XmlStringUtil;
import consulo.versionControlSystem.FilePath;
import consulo.versionControlSystem.VcsException;
import consulo.versionControlSystem.change.*;
Expand Down Expand Up @@ -75,6 +75,8 @@
import jakarta.annotation.Nullable;
import jakarta.inject.Inject;
import jakarta.inject.Singleton;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.swing.*;
import java.awt.*;
Expand All @@ -90,19 +92,18 @@

import static consulo.util.collection.ContainerUtil.*;
import static consulo.util.lang.ObjectUtil.assertNotNull;
import static consulo.util.lang.StringUtil.escapeXml;
import static consulo.versionControlSystem.change.ChangesUtil.getAfterPath;
import static consulo.versionControlSystem.change.ChangesUtil.getBeforePath;
import static consulo.versionControlSystem.distributed.DvcsUtil.getShortRepositoryName;
import static git4idea.GitUtil.getLogString;
import static git4idea.GitUtil.LogString;
import static git4idea.GitUtil.getRepositoryManager;
import static java.util.Arrays.asList;

@Singleton
@ServiceAPI(ComponentScope.PROJECT)
@ServiceImpl
public class GitCheckinEnvironment implements CheckinEnvironment {
private static final Logger LOG = Logger.getInstance(GitCheckinEnvironment.class);
private static final Logger LOG = LoggerFactory.getLogger(GitCheckinEnvironment.class);
private static final String GIT_COMMIT_MSG_FILE_PREFIX = "git-commit-msg-"; // the file name prefix for commit message file
private static final String GIT_COMMIT_MSG_FILE_EXT = ".txt"; // the file extension for commit message file

Expand Down Expand Up @@ -157,7 +158,7 @@ public String getDefaultMessageFor(FilePath[] filesToCheckin) {
for (VirtualFile root : GitUtil.gitRoots(asList(filesToCheckin))) {
GitRepository repository = manager.getRepositoryForRoot(root);
if (repository == null) { // unregistered nested submodule found by GitUtil.getGitRoot
LOG.warn("Unregistered repository: " + root);
LOG.warn("Unregistered repository: {}", root);
continue;
}
File mergeMsg = repository.getRepositoryFiles().getMergeMessageFile();
Expand All @@ -175,9 +176,7 @@ public String getDefaultMessageFor(FilePath[] filesToCheckin) {
}
}
catch (IOException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Unable to load merge message", e);
}
LOG.debug("Unable to load merge message", e);
}
}
return DvcsUtil.joinMessagesOrNull(messages);
Expand Down Expand Up @@ -208,7 +207,9 @@ public List<VcsException> commit(
) {
List<VcsException> exceptions = new ArrayList<>();
Map<VirtualFile, Collection<Change>> sortedChanges = sortChangesByGitRoot(changes, exceptions);
LOG.assertTrue(!sortedChanges.isEmpty(), "Trying to commit an empty list of changes: " + changes);
if (sortedChanges.isEmpty()) {
LOG.error("Trying to commit an empty list of changes: {}", changes);
}
for (Map.Entry<VirtualFile, Collection<Change>> entry : sortedChanges.entrySet()) {
VirtualFile root = entry.getKey();
File messageFile;
Expand Down Expand Up @@ -279,7 +280,7 @@ public List<VcsException> commit(
}
finally {
if (!messageFile.delete()) {
LOG.warn("Failed to remove temporary file: " + messageFile);
LOG.warn("Failed to remove temporary file: {}", messageFile);
}
}
}
Expand Down Expand Up @@ -316,15 +317,14 @@ private List<VcsException> commitWithCaseOnlyRename(
) {
String rootPath = root.getPath();
LOG.info(
"Committing case only rename: " + getLogString(rootPath, caseOnlyRenames) + " in " +
getShortRepositoryName(project, root)
"Committing case only rename: {} in {}", new LogString(rootPath, caseOnlyRenames), getShortRepositoryName(project, root)
);

// 1. Check what is staged besides case-only renames
Collection<Change> stagedChanges;
try {
stagedChanges = GitChangeUtils.getStagedChanges(project, root);
LOG.debug("Found staged changes: " + getLogString(rootPath, stagedChanges));
LOG.debug("Found staged changes: {}", new LogString(rootPath, stagedChanges));
}
catch (VcsException e) {
return Collections.singletonList(e);
Expand All @@ -338,7 +338,7 @@ private List<VcsException> commitWithCaseOnlyRename(
&& !removed.contains(getBeforePath(change))
);
if (!excludedStagedChanges.isEmpty()) {
LOG.info("Staged changes excluded for commit: " + getLogString(rootPath, excludedStagedChanges));
LOG.info("Staged changes excluded for commit: {}", new LogString(rootPath, excludedStagedChanges));
try {
reset(project, root, excludedStagedChanges);
}
Expand All @@ -351,7 +351,7 @@ private List<VcsException> commitWithCaseOnlyRename(
try {
// 3. Stage what else is needed to commit
List<FilePath> newPathsOfCaseRenames = map(caseOnlyRenames, ChangesUtil::getAfterPath);
LOG.debug("Updating index for added:" + added + "\n, removed: " + removed + "\n, and case-renames: " + newPathsOfCaseRenames);
LOG.debug("Updating index for added: {}\n, removed: {}\n, and case-renames: {}", added, removed, newPathsOfCaseRenames);
Set<FilePath> toAdd = new HashSet<>(added);
toAdd.addAll(newPathsOfCaseRenames);
updateIndex(project, root, toAdd, removed, exceptions);
Expand All @@ -371,7 +371,7 @@ private List<VcsException> commitWithCaseOnlyRename(
finally {
// 5. Stage back the changes unstaged before commit
if (!excludedStagedChanges.isEmpty()) {
LOG.debug("Restoring changes which were unstaged before commit: " + getLogString(rootPath, excludedStagedChanges));
LOG.debug("Restoring changes which were unstaged before commit: {}", new LogString(rootPath, excludedStagedChanges));
Set<FilePath> toAdd = map2SetNotNull(excludedStagedChanges, ChangesUtil::getAfterPath);
Predicate<Change> isMovedOrDeleted =
change -> change.getType() == Change.Type.MOVED || change.getType() == Change.Type.DELETED;
Expand Down Expand Up @@ -808,7 +808,7 @@ private class GitCheckinOptions implements CheckinChangeListSpecificComponent, R
@Nonnull
private String getToolTip(@Nonnull Project project, @Nonnull CheckinProjectPanel panel) {
VcsUser user = getFirstItem(mapNotNull(panel.getRoots(), it -> GitUserRegistry.getInstance(project).getUser(it)));
String signature = user != null ? escapeXml(VcsUserUtil.toExactString(user)) : "";
String signature = user != null ? XmlStringUtil.escapeText(VcsUserUtil.toExactString(user)) : "";
return "<html>Adds the following line at the end of the commit message:<br/>" +
"Signed-off by: " + signature + "</html>";
}
Expand Down
8 changes: 4 additions & 4 deletions plugin/src/main/java/git4idea/reset/GitNewResetDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import static consulo.versionControlSystem.distributed.DvcsUtil.getShortRepositoryName;

public class GitNewResetDialog extends DialogWrapper {

private static final String DIALOG_ID = "git.new.reset.dialog";

@Nonnull
Expand Down Expand Up @@ -123,8 +122,10 @@ private static String prepareDescription(@Nonnull Project project, @Nonnull Map<

@Nonnull
private static String getTargetText(@Nonnull VcsFullCommitDetails commit) {
String commitMessage = StringUtil.escapeXml(StringUtil.shortenTextWithEllipsis(commit.getSubject(), 20, 0));
return String.format("<code><b>%s</b> \"%s\"</code> by <code>%s</code>", commit.getId().toShortString(), commitMessage,
String commitMessage = XmlStringUtil.escapeText(StringUtil.shortenTextWithEllipsis(commit.getSubject(), 20, 0));
return String.format("<code><b>%s</b> \"%s\"</code> by <code>%s</code>",
commit.getId().toShortString(),
commitMessage,
commit.getAuthor().getName()
);
}
Expand All @@ -147,5 +148,4 @@ private static boolean isMultiRepo(@Nonnull Project project) {
public GitResetMode getResetMode() {
return myEnumModel.getSelected();
}

}
10 changes: 7 additions & 3 deletions plugin/src/main/java/git4idea/ui/StashInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import consulo.git.localize.GitLocalize;
import consulo.localize.LocalizeValue;
import consulo.util.lang.StringUtil;
import consulo.util.lang.xml.XmlStringUtil;

/**
* Information about one stash.
Expand All @@ -28,11 +28,15 @@ public class StashInfo {
private final String myMessage;
private final LocalizeValue myText; // The formatted text representation

public StashInfo(final String stash, final String branch, final String message) {
public StashInfo(String stash, String branch, String message) {
myStash = stash;
myBranch = branch;
myMessage = message;
myText = GitLocalize.unstashStashesItem(StringUtil.escapeXml(stash), StringUtil.escapeXml(branch), StringUtil.escapeXml(message));
myText = GitLocalize.unstashStashesItem(
XmlStringUtil.escapeText(stash),
XmlStringUtil.escapeText(branch),
XmlStringUtil.escapeText(message)
);
}

@Override
Expand Down
Loading