#1788: Created a commandlet to simulate the behaviour of ln -s#1847
#1788: Created a commandlet to simulate the behaviour of ln -s#1847KarimALotfy wants to merge 2 commits intodevonfw:mainfrom
Conversation
Added Unit Test for LnCommandlet Class
Coverage Report for CI Build 24888645898Coverage decreased (-0.2%) to 70.483%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions11 previously-covered lines in 2 files lost coverage.
Coverage Stats
💛 - Coveralls |
|
|
jakozian
left a comment
jakozian
left a comment
There was a problem hiding this comment.
Nice approach! But i requested changes because most things regarding the issue can be done with FileAccess/FileAccessImpl.
| try { | ||
| Files.createSymbolicLink(targetPath, sourcePath); | ||
| LOG.info("Created symbolic link {} -> {}", targetPath, sourcePath); | ||
| return; |
There was a problem hiding this comment.
This return statement is unnecessary because it is the last statement in this method if symlink creation succeeds.
| ensureSameVolume(sourcePath, targetPath); | ||
|
|
||
| try { | ||
| Files.createLink(targetPath, sourcePath); |
There was a problem hiding this comment.
For file access and operations look into this.context.getFilesAccess().symlink... (FileAccess & FileAccessImpl) because most of the handling of symlinks, junctions and hardlinks is already done there.
With that most of your code becomes obsolete and everything can be shortened.
| String msg = fse.getMessage(); | ||
| if (msg != null) { | ||
| String m = msg.toLowerCase(); | ||
| return m.contains("required privilege") |
There was a problem hiding this comment.
These contains are not locale safe. E.g. when i run the code on my german machine the error does not get recognized because the message is german.
But as suggested above, this also gets handled by FileAccess of IDEasy.
| public final KeywordProperty symbolic; | ||
|
|
||
| /** The source path to link to. */ | ||
| public final StringProperty source; |
There was a problem hiding this comment.
IMO it would be better to conform to the syntax and options of the actual ln -s command:
ln -s <target> <link> instead of ln -s <source> <target>
4 participants
This PR fixes #1788
Implemented changes:
Added ide ln -s commandlet to create file links.
Checklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)with
internal