-
-
Notifications
You must be signed in to change notification settings - Fork 58
Ticket #4861: add extension-based fallbacks for handling JAR and WAR files to mc.ext #4862
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i find the whole approach mildly fishy; see the inline comment.
the lines in the commit message are a bit long.
misc/mc.ext.ini.in
Outdated
|
|
||
| [jar-by-shell] | ||
| Shell=.jar | ||
| TypeIgnoreCase=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's the wrong keyword here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep 👍
misc/mc.ext.ini.in
Outdated
| View=%view{ascii} @EXTHELPERSDIR@/archive.sh view zip | ||
|
|
||
| [war-by-shell] | ||
| Shell=.war |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than duplicating the whole thing, a regex match would be preferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the submitter predictably disappeared, as it usually turns out to be the case, I have updated the code myself.
|
|
||
| [jar] | ||
| [jar-by-type] | ||
| Type=\\(Java (Jar file|archive) data \\((zip|JAR)\\)\\) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
based on the addition of .war handling below, it would seem logical to add it here as well, or does no file variant recognize these as anything other than a regular zip archive?
actually, what are the files recognized as? you're not saying in the commit message.
the idea behind using content based detection is to catch files with non-standard extensions (for .jars, that may be modules in some applications). falling back to extension based detection is basically admitting defeat ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understand, wars are nothing other than jars, only meant to be uploaded to the application servers, so file output would be the same for both. In this sense, they have always been implicitly supported, or at least they should have been.
Given a number of recent posts to #4862 tickets, however, I think indeed it's time to admit defeat and add extension-based fallbacks. It seems that the problem is that even though we are now correctly matching on the file output, the utility itself is unreliable and often enough doesn't detect the correct type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.wars contain additional metadata, so they could be detected separately.
i'd really like to see the exact status quo documented (in the commit message), so it's obvious what is being worked around and that this is the only reasonable way to do it.
|
Why not identify files by content when extension associations search fails? Like Far Manager does. |
This PR does exactly the opposite: identify by extension if content search fails (which apparently happens for a surprisingly high number of users even after all the fixes that we've made to it). |
…ling JAR and WAR files to mc.ext The `file` command apparently behaves inconsistently between Linux distributions—that prevents opening JAR files (which are just ZIP files with metadata) in Manjaro/Arch Linux within Midnight Commander; and inspecting/editing source code within those compressed files. Signed-off-by: Gerard Bosch <30733556+gerardbosch@users.noreply.github.com> Signed-off-by: Yury V. Zaytsev <yury@shurup.com>
| View=%view{ascii} @EXTHELPERSDIR@/archive.sh view zip | ||
|
|
||
| [jar-war-by-regex] | ||
| Regex=\\.(j|w)ar$ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could use [jw] instead of (j|w). i have no preference.
|
|
||
| [jar] | ||
| [jar-by-type] | ||
| Type=\\(Java (Jar file|archive) data \\((zip|JAR)\\)\\) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.wars contain additional metadata, so they could be detected separately.
i'd really like to see the exact status quo documented (in the commit message), so it's obvious what is being worked around and that this is the only reasonable way to do it.
Proposed changes
Fix/add support for JAR/WAR files with shell associations.
The
filecommand apparently behaves inconsistently between Linux distributions—that prevents opening JAR files (which are just ZIP files with metadata) in Manjaro/Arch Linux within Midnight Commander; and inspecting/editing source code within those compressed files.WAR files were missing (that was reported here): https://bugs.launchpad.net/mc/+bug/2062968/comments/16 - this PR adds support for them via extension match.
.jarfiles:fileoutput doesn't match & missing[jar-by-shell]#4861Checklist
👉 Our coding style can be found here: https://midnight-commander.org/coding-style/ 👈
git commit --amend -smake indent && make check)