Feature: Implementation of Quick Access API for Windows Explorer #69
Conversation
generated with jextract
# Conflicts: # pom.xml
WalkthroughThe 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
Poem
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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 SpecifiedThe service file correctly specifies
ExplorerQuickAccessServiceas the implementation forQuickAccessService. 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 ValuesThe
RegistryValueExceptionclass is well-implemented, extendingRegistryKeyExceptionwith 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 OperationsThe
RegistryKeyExceptionclass 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 ServiceThe test method
testExplorerSidebarIntegrationis 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 ServiceThe module configuration correctly declares
ExplorerQuickAccessServiceas the provider forQuickAccessService. 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
findOrThrowthrows anUnsatisfiedLinkErrorif 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 inGetLastError.Ensure that the error handling in
GetLastErroris 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
GetLastErrorfunction is invoked multiple times inWindowsRegistry.java, and in each case, the error is appropriately handled by throwing aWindowsExceptionwith relevant context. This indicates that the error handling forGetLastErroris adequate and all potential error conditions are managed properly.
WindowsRegistry.java, lines related toGetLastErrorinvocations:
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.javaLength of output: 1797
src/main/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessService.java (1)
27-106: Ensure robust parameter validation and error handling inaddmethod.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
addmethod.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 5Length 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 andjextractplugin version updates.Updating the API version to
1.4.0-beta1ensures compatibility with the latest changes in the related API project. The addition ofjextractdependencies 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 forjextract.The specified paths and executable settings are crucial for the correct functioning of the
jextractplugin. Ensure these paths are maintained and updated as necessary to reflect the actual environment.
401-521: ReviewjextractMaven profile configuration.The
jextractprofile 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
jextractMaven 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 thepom.xmlfor their definitions and ensure they are correctly set.
Review
jextractMaven profile configuration.The Maven properties
<win.umHeaderPath>and<jextract.executable>are correctly defined in thepom.xmlfile:
<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.batThese 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." fiLength 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.xmlLength 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
aligncustomizes 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.
| @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); | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
| @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); | |
| } | |
| } |
src/main/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessService.java
Outdated
Show resolved
Hide resolved
src/main/java/org/cryptomator/windows/quickaccess/ExplorerQuickAccessService.java
Show resolved
Hide resolved
Co-authored-by: Sebastian Stenzel <overheadhunter@users.noreply.github.com>
There was a problem hiding this comment.
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 apiTools
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
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:WindowsRegistryThe class
WindowsRegistryis 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 instartTransaction.The method correctly handles errors by throwing a
WindowsExceptionif 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 increateRegKey.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 inopenRegKey.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 indeleteRegKey.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 incommit.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 inrollback.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 inclose.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 incloseInternal.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 constructorRegistryTransaction.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)
| @@ -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: | |||
There was a problem hiding this comment.
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.
| 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)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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
src/main/java/org/cryptomator/windows/common/WindowsRegistry.java
Outdated
Show resolved
Hide resolved
src/main/java/org/cryptomator/windows/common/WindowsRegistry.java
Outdated
Show resolved
Hide resolved
src/main/java/org/cryptomator/windows/common/WindowsRegistry.java
Outdated
Show resolved
Hide resolved
* 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
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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
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.
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
Note, that due to the FF API, service consumers must add the jvm Option "--enable-native-access=org.cryptomator.integrations.win.quickaccess"