Skip to content
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

#1008: improve upgrade settings #1146

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
+51 −0
Open

#1008: improve upgrade settings #1146

Changes from 14 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
68623eb
#999: More flexible IDELogEntry comparison
leonrohne27 Mar 13, 2025
ee0472a
Merge branch 'main' into fix/999-make-idelogentry-comparing-more-flex…
leonrohne27 Mar 13, 2025
9dcfdc1
#1130: Improve behaviour on ambigous xpath match
leonrohne27 Mar 17, 2025
f832220
fix
KianRolf Mar 17, 2025
49ef8ef
Update IdeTestLoggerAssertion.java
KianRolf Mar 17, 2025
0dcb661
Update IdeTestLoggerAssertion.java
leonrohne27 Mar 17, 2025
e37e0b4
Merge branch 'main' into fix/1130-improve-behaviour-on-ambigous-xpath…
leonrohne27 Mar 17, 2025
c359522
#
leonrohne27 Mar 18, 2025
2c30607
Merge branch 'main' into fix/1130-improve-behaviour-on-ambigous-xpath…
leonrohne27 Mar 18, 2025
e8a9fc0
#
leonrohne27 Mar 18, 2025
be644c6
Merge branch 'fix/1130-improve-behaviour-on-ambigous-xpath-match' of …
leonrohne27 Mar 18, 2025
255542a
#1008: improve upgrade-settings commandlet
leonrohne27 Mar 19, 2025
9610ea9
#fixed branch issues
leonrohne27 Mar 19, 2025
2dfc249
Merge branch 'main' into fix/1008-improve-upgrade-settings
leonrohne27 Mar 20, 2025
6345aa9
Merge branch 'main' into fix/1008-improve-upgrade-settings
leonrohne27 Mar 21, 2025
ad54840
Update cli/src/main/java/com/devonfw/tools/ide/commandlet/UpgradeSett…
leonrohne27 Mar 21, 2025
f890489
Update cli/src/main/java/com/devonfw/tools/ide/commandlet/UpgradeSett…
leonrohne27 Mar 21, 2025
e17e2b5
Update cli/src/main/java/com/devonfw/tools/ide/commandlet/UpgradeSett…
leonrohne27 Mar 21, 2025
2c26a35
#1008: implemented suggested changes
leonrohne27 Mar 25, 2025
e6105ad
Merge branch 'main' into fix/1008-improve-upgrade-settings
leonrohne27 Mar 25, 2025
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
45 changes: 45 additions & 0 deletions cli/src/main/java/com/devonfw/tools/ide/commandlet/UpgradeSettingsCommandlet.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
package com.devonfw.tools.ide.commandlet;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Function;

import com.devonfw.tools.ide.context.IdeContext;
@@ -139,6 +144,7 @@ private void updateProperties(EnvironmentVariablesPropertiesFile environmentVari
}
updatePropertiesLegacyEdition(environmentVariables, "INTELLIJ_EDITION_TYPE", "INTELLIJ_EDITION", this::mapLegacyIntellijEdition);
updatePropertiesLegacyEdition(environmentVariables, "ECLIPSE_EDITION_TYPE", "ECLIPSE_EDITION", this::mapLegacyEclipseEdition);
cleanupLegacyProperties();
environmentVariables.save();
this.context.getFileAccess().backup(environmentVariables.getLegacyPropertiesFilePath());
}
@@ -181,4 +187,43 @@ private static void updatePropertiesLegacyEdition(EnvironmentVariablesProperties
environmentVariables.remove(legacyEditionVariable);
}
}

private void cleanupLegacyProperties() {
this.context.info("Cleaning up legacy properties...");

Path rootDirectory = Paths.get(System.getProperty("user.dir"));
Copy link
Member

@hohwille hohwille Mar 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont hardcode user.dir this will cause JUnits to apply the migration in your home directory but tests should only work in test directories (./target/) and should never cause such side-effects.
Also the variable name could be named more intuitive:

Suggested change
Path rootDirectory = Paths.get(System.getProperty("user.dir"));
Path homeDirectory = this.config.getgetUserHome();

However, why are you searching recursively for IDEasy.properties files in the users home directory?
This does not make sense.

What you IMHO actually need is something like this:

    Path settingsPath = context.getSettingsPath();
    Path repositoriesPath = settingsPath.resolve(IdeContext.FOLDER_REPOSITORIES);
    if (!Files.isDirectory(repositoriesPath)) {
      return;
    }
    try (Stream<Path> childStream = Files.list(repositoriesPath)) {
      Iterator<Path> iterator = childStream.iterator();
      while (iterator.hasNext()) {
        Path child = iterator.next();
        if (child.getFileName().toString().endsWith(IdeContext.EXT_PROPERTIES);
        updateProperties(child);
      }
    }

Copy link
Member

@hohwille hohwille Mar 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW: Instead of using Files.list directly what also requires a catch statement that I omitted, it seems smarter that you use fileAccess like already done in the same class here:

IDEasy/cli/src/main/java/com/devonfw/tools/ide/commandlet/UpgradeSettingsCommandlet.java

Lines 83 to 89 in 8a9936e

fileAccess.listChildrenMapped(settingsDir, child -> {
Path childWorkspaceDir = child.resolve(IdeContext.FOLDER_WORKSPACE);
if (fileAccess.isExpectedFolder(childWorkspaceDir)) {
merger.upgrade(childWorkspaceDir);
}
return null;
});

This allows to supply a lambda function that is called for each file found in the directory (you can configure if only flat or recursively). That lambda function is used to map and filter the found child Path objects. When it returns null the child is filtered and not returned. You could even map if from Path to some other Java type like String in this function. By directly applying the modification as "side-effect" inside the lambda function the result of the listChildrenMapped method can be ignored. In the example code the side effect is merger.upgrade and in your case it would be updateProperties instead.

try {
Files.walk(rootDirectory)
.filter(path -> path.getFileName().toString().equals("IDEasy.properties"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obsolete but for the record. This filename IDEasy.properties is just an example and cannot be hardcoded.
There can be any number of *.properites files in the repositories folder and you cannot make any assumptions about their name.

.forEach(filePath -> {
try {
updateProperties(filePath);
} catch (IOException e) {
this.context.warning("Error processing IDEasy.properties at " + filePath);
}
});
} catch (IOException e) {
this.context.warning("Error walking the file tree to find IDEasy.properties.");
}
}

private void updateProperties(Path filePath) throws IOException {
List<String> lines = Files.readAllLines(filePath);
List<String> updatedLines = new ArrayList<>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: You can do the modifications directly in the original list (lines) and do not need a second list as a copy.


for (String line : lines) {
if (line.startsWith("git.url=") || line.startsWith("git-url")) {
String gitUrl = line.substring("git.url=".length());
updatedLines.add("git_url=" + gitUrl);
continue;
}
if (line.startsWith("eclipse=import")) {
updatedLines.add("import=eclipse");
continue;
}
updatedLines.add(line);
}
Files.write(filePath, updatedLines, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should add a boolean flag boolean updated=false; before the loop and set it to true when you have modified something.
Then you can only write the file changes back if something changed and additionally log the success message only in that case so if nothing changed then nothing if logged.

this.context.success("Successfully updated IDEasy.properties at " + filePath);
}
}