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.

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

Feature: Implementation of Quick Access API for Windows Explorer #69

Merged
merged 49 commits into from
Jul 12, 2024

Conversation

infeo
Copy link
Member

@infeo infeo commented Jun 27, 2024

This PR adds an implementation of the cryptomator/integrations-api#36 for Windows Explorer.

This implementation defines the quick access area as the Explorer navigation pane. The section allows a custom label for the entry and a custom icon. The icon is not configurable, it is either the same as the executing command or, if the command path is not present, the default windows directory icon.
grafik

In general the implemenation follows a Microsoft example. The code is enhanced by enclosing any registry operation within a transaction, such that on any failure, the changes are reverted.

Note: In order to use native Windows APIs, this PR additionally

  • updates project to JDK 22
  • uses the ForeignFunction API
  • adds jextract goal to the maven project

Note, that due to the FF API, service consumers must add the jvm Option "--enable-native-access=org.cryptomator.integrations.win.quickaccess"

infeo added 30 commits June 12, 2024 15:57
generated with jextract
Copy link

coderabbitai bot commented Jun 27, 2024

Walkthrough

The updates focus on upgrading the project’s Java version from 17 to 22 across various GitHub Actions workflows, project configurations, and documentation. Additionally, significant new functionalities have been introduced, such as enhanced Windows registry interaction through transaction-based operations and native bindings for Windows APIs. These changes also include updating dependencies, adding new classes and methods for Windows API interactions, and integrating a new quick access service for Windows Explorer.

Changes

File(s) Change Summary
.github/workflows/build.yml, .github/workflows/codeql-analysis.yml, .github/workflows/dependency-check.yml, .github/workflows/publish-... Updated Java version from 17 to 22 in GitHub Actions workflows.
.idea/misc.xml Changed the languageLevel attribute in the ProjectRootManager component from JDK_17 to JDK_22.
pom.xml Updated Java version to 22, API version to 1.4.0-beta2, added jextract-maven dependencies, and configured jextract properties for Windows Kits.
src/main/java/module-info.java Added QuickAccessService and ExplorerQuickAccessService to the module.
src/main/java/org/cryptomator/windows/capi/common/Windows_h.java Added methods and constants for Windows API function bindings.
src/main/java/org/cryptomator/windows/capi/ktmw32/Ktmw32_h.java Introduced Java bindings for the Windows Kernel Transaction Manager.
src/main/java/org/cryptomator/windows/capi/winreg/Winreg_h.java Added native methods for Windows Registry interactions.
src/main/java/org/cryptomator/windows/common/WindowsRegistry.java Added functionality for managing Windows Registry transactions.
src/main/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessService.java Implemented QuickAccessService for Windows Explorer, allowing manipulation of the Explorer navigation pane.
src/main/java/org/cryptomator/windows/common/RegistryKey.java Provided methods for interacting with Windows registry keys.
src/main/resources/META-INF/services/org.cryptomator.integrations.quickaccess.QuickAccessService Added ExplorerQuickAccessService to service providers.
src/main/resources/ktmw32_helper.h Introduced includes for Windows API and ktmw32 library.
src/test/java/org/cryptomator/windows/common/WindowsRegistryIT.java Added tests for Windows registry operations and transaction handling.
README.md Updated required JDK version from 17 to 22 in the project setup instructions.
.coderabbit/config.yml Introduced a configuration change related to path filters, excluding a certain directory in the Java source code.

Poem

Upgrading Java, a leap to new heights,
From 17 to 22, our code ignites,
Registry keys now dance and sing,
Transactions roll back, commit, and bring,
Quick access to Explorer, a swift delight,
With bindings and tests, everything's right!
Oh, what a change, a coder's delight! 🌟🛠️


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f8aef6c and 6f490e8.

Files selected for processing (21)
  • .github/workflows/build.yml (1 hunks)
  • .github/workflows/codeql-analysis.yml (1 hunks)
  • .github/workflows/dependency-check.yml (1 hunks)
  • .github/workflows/publish-central.yml (1 hunks)
  • .github/workflows/publish-github.yml (1 hunks)
  • .idea/misc.xml (2 hunks)
  • pom.xml (3 hunks)
  • src/main/java/module-info.java (2 hunks)
  • src/main/java/org/cryptomator/windows/capi/common/Windows_h.java (1 hunks)
  • src/main/java/org/cryptomator/windows/capi/ktmw32/Ktmw32_h.java (1 hunks)
  • src/main/java/org/cryptomator/windows/capi/winreg/Winreg_h.java (1 hunks)
  • src/main/java/org/cryptomator/windows/common/RegistryKey.java (1 hunks)
  • src/main/java/org/cryptomator/windows/common/RegistryKeyException.java (1 hunks)
  • src/main/java/org/cryptomator/windows/common/RegistryValueException.java (1 hunks)
  • src/main/java/org/cryptomator/windows/common/WindowsException.java (1 hunks)
  • src/main/java/org/cryptomator/windows/common/WindowsRegistry.java (1 hunks)
  • src/main/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessService.java (1 hunks)
  • src/main/resources/META-INF/services/org.cryptomator.integrations.quickaccess.QuickAccessService (1 hunks)
  • src/main/resources/ktmw32_helper.h (1 hunks)
  • src/test/java/org/cryptomator/windows/common/WindowsRegistryIT.java (1 hunks)
  • src/test/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessServiceIT.java (1 hunks)
Files not reviewed due to errors (4)
  • src/main/java/org/cryptomator/windows/common/WindowsRegistry.java (no review received)
  • src/main/java/org/cryptomator/windows/common/RegistryKey.java (no review received)
  • src/test/java/org/cryptomator/windows/common/WindowsRegistryIT.java (no review received)
  • src/main/java/org/cryptomator/windows/capi/ktmw32/Ktmw32_h.java (no review received)
Files skipped from review due to trivial changes (5)
  • .github/workflows/codeql-analysis.yml
  • .github/workflows/dependency-check.yml
  • .idea/misc.xml
  • src/main/java/org/cryptomator/windows/common/WindowsException.java
  • src/main/resources/ktmw32_helper.h
Additional comments not posted (16)
src/main/resources/META-INF/services/org.cryptomator.integrations.quickaccess.QuickAccessService (1)

1-1: Correct Service Implementation Specified

The service file correctly specifies ExplorerQuickAccessService as the implementation for QuickAccessService. This setup is essential for the service loader to function properly.

src/main/java/org/cryptomator/windows/common/RegistryValueException.java (1)

1-8: Proper Exception Handling for Registry Values

The RegistryValueException class is well-implemented, extending RegistryKeyException with specific handling for registry value errors. The constructor's use of string concatenation to enhance the error message is appropriate.

src/main/java/org/cryptomator/windows/common/RegistryKeyException.java (1)

1-11: Well-Defined Base Exception for Registry Operations

The RegistryKeyException class correctly encapsulates information about registry operations errors, including the key path involved. This design aids in debugging and error tracking.

src/test/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessServiceIT.java (1)

1-34: Integration Test for Quick Access Service

The test method testExplorerSidebarIntegration is well-structured but currently disabled. Ensure to enable it when the testing environment is properly set up and all dependencies are met. It's crucial to verify the integration thoroughly in different scenarios.

src/main/java/module-info.java (1)

Line range hint 1-24: Module Configuration for Quick Access Service

The module configuration correctly declares ExplorerQuickAccessService as the provider for QuickAccessService. This is essential for the correct operation of service loading mechanisms and for ensuring that the new functionality is accessible as intended.

.github/workflows/publish-central.yml (1)

24-24: Java version update approved.

Upgrading to Java 22 aligns with the project's overall upgrade strategy.

.github/workflows/build.yml (1)

21-21: Java version update approved.

The update to Java 22 is consistent across workflows, ensuring compatibility with the JDK upgrade.

.github/workflows/publish-github.yml (1)

19-19: Java version update approved.

Upgrading to Java 22 is consistent across all deployment and build workflows.

src/main/java/org/cryptomator/windows/capi/common/Windows_h.java (2)

31-34: Ensure robust symbol resolution.

The method findOrThrow throws an UnsatisfiedLinkError if a symbol cannot be found. Ensure that all symbols passed to this method are expected to exist to avoid runtime errors. Additionally, consider logging the missing symbol for easier debugging.


176-186: Validate error handling in GetLastError.

Ensure that the error handling in GetLastError is adequate and that all potential error conditions are appropriately managed. Consider adding specific error handling cases if needed.

Verification successful

Validate error handling in GetLastError.

The GetLastError function is invoked multiple times in WindowsRegistry.java, and in each case, the error is appropriately handled by throwing a WindowsException with relevant context. This indicates that the error handling for GetLastError is adequate and all potential error conditions are managed properly.

  • WindowsRegistry.java, lines related to GetLastError invocations:
    • startTransaction()
    • CommitTransaction()
    • RollbackTransaction()
    • closeInternal()
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Check usage of GetLastError to ensure error handling is implemented
rg --type java --files-with-matches $'GetLastError' | xargs -L 1 ast-grep --lang java --pattern 'GetLastError()'

Length of output: 111



Script:

#!/bin/bash
# Search for the definition of GetLastError function and its usages in the codebase to verify error handling.
echo "Searching for the definition of GetLastError function:"
ast-grep --lang java --pattern 'int GetLastError()' --json

echo "Searching for usages of GetLastError function in the codebase:"
rg --type java 'GetLastError()'

Length of output: 2490



Script:

#!/bin/bash
# Extract code snippets from WindowsRegistry.java where GetLastError is invoked to verify error handling.
echo "Extracting code snippets from WindowsRegistry.java where GetLastError is invoked:"
rg --type java -A 5 -B 5 'GetLastError()' src/main/java/org/cryptomator/windows/common/WindowsRegistry.java

Length of output: 1797

src/main/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessService.java (1)

27-106: Ensure robust parameter validation and error handling in add method.

The method correctly checks for null parameters but consider enhancing the error messages to include which parameter is null for better debugging. Also, ensure that all registry operations are wrapped in appropriate error handling to prevent partial updates in case of failures.

- throw new IllegalArgumentException("Parameter 'displayname' must not be null.");
+ throw new IllegalArgumentException("Parameter 'displayName' must not be null.");
- throw new IllegalArgumentException("Parameter 'target' must not be null.");
+ throw new IllegalArgumentException("Parameter 'target' must not be null.");
Verification successful

Ensure robust parameter validation and error handling in add method.

The method correctly checks for null parameters but consider enhancing the error messages to include which parameter is null for better debugging. The registry operations are already wrapped in appropriate error handling mechanisms using try-with-resources and a catch block for WindowsException.

  • Enhance the error messages for null parameters:
    - throw new IllegalArgumentException("Parameter 'displayname' must not be null.");
    + throw new IllegalArgumentException("Parameter 'displayName' must not be null.");
    - throw new IllegalArgumentException("Parameter 'target' must not be null.");
    + throw new IllegalArgumentException("Parameter 'target' must not be null.");
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Check all registry operations for proper error handling
rg --type java --files-with-matches $'RegistryKey' | xargs -L 1 ast-grep --lang java --pattern 'try ($_ = $RegistryKey($_)) { $$$ }'

Length of output: 131



Script:

#!/bin/bash
# Search for all instances of RegistryKey and their surrounding try-catch blocks
rg --type java 'RegistryKey' -A 5 -B 5

Length of output: 45100

pom.xml (4)

37-37: Approved JDK version update.

The update to JDK 22 aligns with modern Java practices and ensures compatibility with the latest features and security enhancements.


40-40: Approved API and jextract plugin version updates.

Updating the API version to 1.4.0-beta1 ensures compatibility with the latest changes in the related API project. The addition of jextract dependencies is essential for working with native code, which is a part of this project's new features.

Also applies to: 51-55


53-55: Approved addition of global properties for jextract.

The specified paths and executable settings are crucial for the correct functioning of the jextract plugin. Ensure these paths are maintained and updated as necessary to reflect the actual environment.


401-521: Review jextract Maven profile configuration.

The jextract profile is well-defined with specific goals and configurations tailored for extracting Windows API bindings. Ensure that the paths and macros specified are correct and accessible on the build environment to avoid build failures.

Consider making paths and executable settings configurable through environment variables or properties to enhance portability and flexibility.

Verification successful

Review jextract Maven profile configuration.

The variables ${win.umHeaderPath} and ${jextract.executable} are Maven properties, not environment variables, causing the shell script to fail. To verify these properties, we need to inspect the pom.xml for their definitions and ensure they are correctly set.


Review jextract Maven profile configuration.

The Maven properties <win.umHeaderPath> and <jextract.executable> are correctly defined in the pom.xml file:

  • <win.umHeaderPath>: C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\um
  • <jextract.executable>: C:\Users\Arbeit\Programs\jextract-22\bin\jextract.bat

These paths appear to be valid and specific to the build environment, confirming that the configuration is correct.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the accessibility of the specified paths and macros in the build environment.

# Test: Check if the paths and macros are accessible and valid.
echo "Checking jextract paths and macros..."
if [ -d "${win.umHeaderPath}" ] && [ -x "${jextract.executable}" ]; then
  echo "Paths and executables are correctly set."
else
  echo "Error: One or more paths or executables are not set correctly."
fi

Length of output: 170



Script:

#!/bin/bash
# Description: Verify the definitions of Maven properties used in the jextract profile.

# Search for the definitions of the properties in the pom.xml file.
echo "Checking Maven properties definitions..."
rg '<win.umHeaderPath>|<jextract.executable>' pom.xml

Length of output: 347

src/main/java/org/cryptomator/windows/capi/winreg/Winreg_h.java (1)

44-55: Review the use of custom alignment in memory layouts.

The method align customizes the byte alignment of memory layouts. Ensure that this alignment is necessary and correctly implemented, as incorrect alignment can lead to significant issues in native interactions.

Comment on lines 117 to 147
@Override
public synchronized void remove() throws QuickAccessServiceException {
if (isClosed) {
return;
}

LOG.debug("Removing navigation pane entry with CLSID {}", clsid);
try (var t = WindowsRegistry.startTransaction()) {
//undo step 11.
var nameSpaceSubkey = "Software\\Microsoft\\Windows\\CurrentVersion\\Explorer\\Desktop\\NameSpace\\" + clsid;
LOG.trace("Removing RegKey {}", nameSpaceSubkey);
t.deleteRegKey(RegistryKey.HKEY_CURRENT_USER, nameSpaceSubkey, true);

//undo step 12.
try (var nameSpaceKey = t.openRegKey(RegistryKey.HKEY_CURRENT_USER, "Software\\Microsoft\\Windows\\CurrentVersion\\Explorer\\HideDesktopIcons\\NewStartPanel")) {
LOG.trace("Removing Value {} of RegKey {}", clsid, nameSpaceKey);
nameSpaceKey.deleteValue(clsid, true);
}

//undo everything else
try (var baseKey = t.openRegKey(RegistryKey.HKEY_CURRENT_USER, "Software\\Classes\\CLSID\\" + clsid)) {
LOG.trace("Wiping everything under RegKey {} and key itself.", baseKey);
baseKey.deleteAllValuesAndSubtrees();
}
t.deleteRegKey(RegistryKey.HKEY_CURRENT_USER, "Software\\Classes\\CLSID\\" + clsid, true);
t.commit();
isClosed = true;
} catch (WindowsException e) {
throw new QuickAccessServiceException("Removing entry from Explorer navigation pane via Windows registry failed.", e);
}
}
Copy link

Choose a reason for hiding this comment

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

Validate cleanup logic in remove method.

The remove method attempts to clean up registry entries, but ensure that each step is necessary and correctly reverts the changes made by add. Also, consider adding more detailed logging to trace each step of the cleanup process.

+ LOG.debug("Starting cleanup of registry entries.");
- LOG.trace("Removing RegKey {}", nameSpaceSubkey);
+ LOG.debug("Removing namespace key: {}", nameSpaceSubkey);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@Override
public synchronized void remove() throws QuickAccessServiceException {
if (isClosed) {
return;
}
LOG.debug("Removing navigation pane entry with CLSID {}", clsid);
try (var t = WindowsRegistry.startTransaction()) {
//undo step 11.
var nameSpaceSubkey = "Software\\Microsoft\\Windows\\CurrentVersion\\Explorer\\Desktop\\NameSpace\\" + clsid;
LOG.trace("Removing RegKey {}", nameSpaceSubkey);
t.deleteRegKey(RegistryKey.HKEY_CURRENT_USER, nameSpaceSubkey, true);
//undo step 12.
try (var nameSpaceKey = t.openRegKey(RegistryKey.HKEY_CURRENT_USER, "Software\\Microsoft\\Windows\\CurrentVersion\\Explorer\\HideDesktopIcons\\NewStartPanel")) {
LOG.trace("Removing Value {} of RegKey {}", clsid, nameSpaceKey);
nameSpaceKey.deleteValue(clsid, true);
}
//undo everything else
try (var baseKey = t.openRegKey(RegistryKey.HKEY_CURRENT_USER, "Software\\Classes\\CLSID\\" + clsid)) {
LOG.trace("Wiping everything under RegKey {} and key itself.", baseKey);
baseKey.deleteAllValuesAndSubtrees();
}
t.deleteRegKey(RegistryKey.HKEY_CURRENT_USER, "Software\\Classes\\CLSID\\" + clsid, true);
t.commit();
isClosed = true;
} catch (WindowsException e) {
throw new QuickAccessServiceException("Removing entry from Explorer navigation pane via Windows registry failed.", e);
}
}
@Override
public synchronized void remove() throws QuickAccessServiceException {
if (isClosed) {
return;
}
LOG.debug("Starting cleanup of registry entries.");
LOG.debug("Removing navigation pane entry with CLSID {}", clsid);
try (var t = WindowsRegistry.startTransaction()) {
//undo step 11.
var nameSpaceSubkey = "Software\\Microsoft\\Windows\\CurrentVersion\\Explorer\\Desktop\\NameSpace\\" + clsid;
LOG.debug("Removing namespace key: {}", nameSpaceSubkey);
t.deleteRegKey(RegistryKey.HKEY_CURRENT_USER, nameSpaceSubkey, true);
//undo step 12.
try (var nameSpaceKey = t.openRegKey(RegistryKey.HKEY_CURRENT_USER, "Software\\Microsoft\\Windows\\CurrentVersion\\Explorer\\HideDesktopIcons\\NewStartPanel")) {
LOG.trace("Removing Value {} of RegKey {}", clsid, nameSpaceKey);
nameSpaceKey.deleteValue(clsid, true);
}
//undo everything else
try (var baseKey = t.openRegKey(RegistryKey.HKEY_CURRENT_USER, "Software\\Classes\\CLSID\\" + clsid)) {
LOG.trace("Wiping everything under RegKey {} and key itself.", baseKey);
baseKey.deleteAllValuesAndSubtrees();
}
t.deleteRegKey(RegistryKey.HKEY_CURRENT_USER, "Software\\Classes\\CLSID\\" + clsid, true);
t.commit();
isClosed = true;
} catch (WindowsException e) {
throw new QuickAccessServiceException("Removing entry from Explorer navigation pane via Windows registry failed.", e);
}
}

Copy link
Member

@overheadhunter overheadhunter left a comment

Choose a reason for hiding this comment

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

Looks very promising. In the long term, it would be a nice addition to outsource the registry part to a separate library (coffeelibs? 😉)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Outside diff range and nitpick comments (1)
README.md (1)

Line range hint 8-8: Add blank lines around lists.

Lists should be surrounded by blank lines for better readability.

-This project uses the following JVM properties:
+This project uses the following JVM properties:

* `cryptomator.integrationsWin.autoStartShellLinkName` - Name of the shell link, which is placed in the Windows startup folder to start application on user login
* `cryptomator.integrationsWin.keychainPaths` - List of file paths, which are checked for data encrypted with the Windows data protection api
Tools
LanguageTool

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6f490e8 and b2856be.

Files selected for processing (5)
  • README.md (1 hunks)
  • pom.xml (3 hunks)
  • src/main/java/org/cryptomator/windows/common/RegistryKey.java (1 hunks)
  • src/main/java/org/cryptomator/windows/common/WindowsRegistry.java (1 hunks)
  • src/main/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessService.java (1 hunks)
Files not reviewed due to errors (1)
  • src/main/java/org/cryptomator/windows/common/WindowsRegistry.java (no review received)
Files skipped from review as they are similar to previous changes (3)
  • pom.xml
  • src/main/java/org/cryptomator/windows/common/RegistryKey.java
  • src/main/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessService.java
Additional context used
LanguageTool
README.md

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

Markdownlint
README.md

8-8: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)

Additional comments not posted (11)
README.md (11)

15-15: Update JDK version.

The JDK version has been updated from 17 to 22.


Line range hint 1-1: Class Definition: WindowsRegistry

The class WindowsRegistry is defined to handle Windows registry operations.

Tools
LanguageTool

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)


Line range hint 20-28: Ensure proper error handling in startTransaction.

The method correctly handles errors by throwing a WindowsException if the transaction handle is invalid.

Tools
LanguageTool

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)


Line range hint 40-62: Ensure proper resource management in createRegKey.

The method uses Arena.ofConfined() to manage memory resources, ensuring proper allocation and deallocation.

Tools
LanguageTool

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)


Line range hint 64-82: Ensure proper resource management in openRegKey.

The method uses Arena.ofConfined() to manage memory resources, ensuring proper allocation and deallocation.

Tools
LanguageTool

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)


Line range hint 84-104: Ensure proper resource management in deleteRegKey.

The method uses Arena.ofConfined() to manage memory resources, ensuring proper allocation and deallocation.

Tools
LanguageTool

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)


Line range hint 107-118: Ensure proper state management in commit.

The method correctly checks the state of the transaction and commits it, handling errors appropriately.

Tools
LanguageTool

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)


Line range hint 120-130: Ensure proper state management in rollback.

The method correctly checks the state of the transaction and rolls it back, handling errors appropriately.

Tools
LanguageTool

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)


Line range hint 134-144: Ensure proper resource management in close.

The method correctly handles closing the transaction, rolling back if not committed, and managing errors.

Tools
LanguageTool

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)


Line range hint 146-156: Ensure proper resource management in closeInternal.

The method correctly handles closing the transaction handle and managing errors.

Tools
LanguageTool

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)


Line range hint 36-38: Ensure proper initialization in the constructor RegistryTransaction.

The constructor correctly initializes the transaction handle.

Tools
LanguageTool

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)

README.md Show resolved Hide resolved
@@ -12,7 +12,7 @@ This project uses the following JVM properties:

This project uses JNI, hence you'll nedd Java as well as GCC build tools:
Copy link

Choose a reason for hiding this comment

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

Typographical error: Correct the sentence structure.

The word "hence" is an adverb and should be separated from the sentence.

-This project uses JNI, hence you'll nedd Java as well as GCC build tools:
+This project uses JNI. Hence, you'll need Java as well as GCC build tools:
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
This project uses JNI, hence you'll nedd Java as well as GCC build tools:
This project uses JNI. Hence, you'll need Java as well as GCC build tools:
Tools
LanguageTool

[typographical] ~13-~13: The word “hence” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...ion api ## Building This project uses JNI, hence you'll nedd Java as well as GCC build t...

(THUS_SENTENCE)

@infeo infeo requested a review from overheadhunter July 1, 2024 16:06
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b2856be and 4c88ab1.

Files selected for processing (1)
  • .coderabbit/config.yml (1 hunks)
Files skipped from review due to trivial changes (1)
  • .coderabbit/config.yml

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 4c88ab1 and 35fcd77.

Files selected for processing (2)
  • src/main/java/org/cryptomator/windows/common/RegistryKey.java (1 hunks)
  • src/test/java/org/cryptomator/windows/common/WindowsRegistryIT.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • src/main/java/org/cryptomator/windows/common/RegistryKey.java
  • src/test/java/org/cryptomator/windows/common/WindowsRegistryIT.java

infeo added 5 commits July 5, 2024 11:38
* remove methods RegistryKey::deleteAllValuesSubtrees and RegistryKey::deleteSubtree
* rename method RegistryKey::deleteValuesSubtrees to deleteTree and make public
* add doc to method
* does not throw anymore
* the handle is always discarded
* use logger
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 35fcd77 and 7037aad.

Files selected for processing (4)
  • src/main/java/org/cryptomator/windows/common/RegistryKey.java (1 hunks)
  • src/main/java/org/cryptomator/windows/common/WindowsRegistry.java (1 hunks)
  • src/main/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessService.java (1 hunks)
  • src/test/java/org/cryptomator/windows/common/WindowsRegistryIT.java (1 hunks)
Files skipped from review as they are similar to previous changes (4)
  • src/main/java/org/cryptomator/windows/common/RegistryKey.java
  • src/main/java/org/cryptomator/windows/common/WindowsRegistry.java
  • src/main/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessService.java
  • src/test/java/org/cryptomator/windows/common/WindowsRegistryIT.java

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7037aad and fcd11c4.

Files selected for processing (1)
  • src/main/java/org/cryptomator/windows/common/RegistryKey.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/main/java/org/cryptomator/windows/common/RegistryKey.java

@infeo infeo requested a review from overheadhunter July 5, 2024 15:46
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between fcd11c4 and e5e4460.

Files selected for processing (2)
  • pom.xml (3 hunks)
  • src/main/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessService.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • pom.xml
  • src/main/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessService.java

@infeo infeo added this to the next milestone Jul 12, 2024
@infeo infeo merged commit ae6180c into develop Jul 12, 2024
4 checks passed
@infeo infeo deleted the feature/quickaccess branch July 12, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants