-
Notifications
You must be signed in to change notification settings - Fork 47
XALANJ-2709 Enable project to be built on JDK 9+ #124
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
Merged
jkesselm
merged 1 commit into
apache:xalan-java-mvn-refactored
from
kriegaex:XALANJ-2709
Nov 18, 2023
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,49 +1,51 @@ | ||
| /** Taglet for Xalan-Java documentation, giving us a standard way to | ||
| indicate when classes are public only because they are shared | ||
| across packages within Xalan code, not because they are intended for use | ||
| by others. Typical: "@xsl.usage internal" | ||
|
|
||
| Technically it might be better to OSGIfy the Xalan code, which | ||
| would also permit demand-loading of only the classes actually being | ||
| used by this execution... but that's an idea for the future. | ||
|
|
||
| This code renders the tag keywords (internal, advanced, experimental) | ||
| into their expanded renderings in the Javadoc. | ||
| */ | ||
| // | ||
| // Source code recreated from xalan2jtaglet.jar by IntelliJ IDEA | ||
| // (powered by FernFlower decompiler) | ||
| // | ||
|
|
||
| package xalan2jtaglet; | ||
|
|
||
| import com.sun.javadoc.Tag; | ||
|
|
||
| /** | ||
| * Taglet for Xalan-Java documentation, giving us a standard way to | ||
| * indicate when classes are public only because they are shared | ||
| * across packages within Xalan code, not because they are intended for use | ||
| * by others. Typical: "@xsl.usage internal" | ||
| * <p> | ||
| * Technically it might be better to OSGIfy the Xalan code, which | ||
| * would also permit demand-loading of only the classes actually being | ||
| * used by this execution... but that's an idea for the future. | ||
| * <p> | ||
| * This code renders the tag keywords (internal, advanced, experimental) | ||
| * into their expanded renderings in the Javadoc. | ||
| * <p> | ||
| * Source code recreated from xalan2jtaglet.jar by IntelliJ IDEA (powered by | ||
| * FernFlower decompiler), then adjusted to JDK 8 taglet API. | ||
| */ | ||
| public class XSLUsage { | ||
| public static final String TAG = "xsl.usage"; | ||
| private static final int INTERNAL = 0; | ||
| private static final int ADVANCED = 1; | ||
| private static final int EXPERIMENTAL = 2; | ||
| private static final int UNSPECIFIED = -1; | ||
| private static final String[] names = new String[]{"internal", "advanced", "experimental"}; | ||
| private static final String[] colours = new String[]{"FF0000", "00FF00", "0000FF"}; | ||
| private static final String[] messages = new String[]{"**For internal use only**", "**For advanced use only**", "**Experimental**"}; | ||
|
|
||
| public XSLUsage() { | ||
| } | ||
| private static final String[] names = new String[]{ | ||
| "internal", "advanced", "experimental" | ||
| }; | ||
| private static final String[] colours = new String[]{ | ||
| "FF0000", "00FF00", "0000FF" | ||
| }; | ||
| private static final String[] messages = new String[]{ | ||
| "**For internal use only**", "**For advanced use only**", "**Experimental**" | ||
| }; | ||
|
|
||
| public static String getHTML(Tag usageTag) { | ||
| int key = getKey(usageTag); | ||
| return key == -1 ? "" : "<i><font size=\"-1\" color=\"#" + colours[key] + "\"> " + messages[key] + "</font></i></DD>\n"; | ||
| return key == -1 | ||
| ? "" | ||
| : "<i><font size=\"-1\" color=\"#" + colours[key] + "\"> " + messages[key] + "</font></i></DD>\n"; | ||
| } | ||
|
|
||
| private static int getKey(Tag usageTag) { | ||
| for (int i = 0; i < names.length; ++i) { | ||
| if (names[i].equals(usageTag.text())) { | ||
| if (names[i].equals(usageTag.text())) | ||
| return i; | ||
| } | ||
| } | ||
|
|
||
| return -1; | ||
| } | ||
| } |
75 changes: 39 additions & 36 deletions
75
xalan2jtaglet/src/main/java/xalan2jtaglet/XSLUsageTag.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,89 +1,92 @@ | ||
| /** Taglet for Xalan-Java documentation, giving us a standard way to | ||
| indicate when classes are public only because they are shared | ||
| across packages within Xalan code, not because they are intended for use | ||
| by others. Typical: "@xsl.usage internal" | ||
|
|
||
| Technically it might be better to OSGIfy the Xalan code, which | ||
| would also permit demand-loading of only the classes actually being | ||
| used by this execution... but that's an idea for the future. | ||
| */ | ||
| // | ||
| // Source code recreated from xalan2jtaglet.jar by IntelliJ IDEA | ||
| // (powered by FernFlower decompiler) | ||
| // | ||
|
|
||
| package xalan2jtaglet; | ||
|
|
||
| import com.sun.javadoc.Tag; | ||
| import com.sun.tools.doclets.Taglet; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
| /** | ||
| * Taglet for Xalan-Java documentation, giving us a standard way to | ||
| * indicate when classes are public only because they are shared | ||
| * across packages within Xalan code, not because they are intended for use | ||
| * by others. Typical: "@xsl.usage internal" | ||
| * <p> | ||
| * Technically it might be better to OSGIfy the Xalan code, which | ||
| * would also permit demand-loading of only the classes actually being | ||
| * used by this execution... but that's an idea for the future. | ||
| * <p> | ||
| * This code renders the tag keywords (internal, advanced, experimental) | ||
| * into their expanded renderings in the Javadoc. | ||
| * <p> | ||
| * Source code recreated from xalan2jtaglet.jar by IntelliJ IDEA (powered by | ||
| * FernFlower decompiler), then adjusted to JDK 8 taglet API. | ||
| */ | ||
| public class XSLUsageTag implements Taglet { | ||
| private static final String HEADER = "Usage:"; | ||
|
|
||
| public XSLUsageTag() {} | ||
|
|
||
| @Override | ||
| public boolean inConstructor() { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean inField() { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean inMethod() { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean inOverview() { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean inPackage() { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean inType() { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isInlineTag() { | ||
| return false; | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return "xsl.usage"; | ||
| } | ||
|
|
||
| public String toString(Tag arg0) { | ||
| return "\n<DT><b>Usage:</b><DD>" + XSLUsage.getHTML(arg0) + "</DD>\n"; | ||
| @Override | ||
| public String toString(Tag tag) { | ||
| return "\n<DT><b>Usage:</b><DD>" + XSLUsage.getHTML(tag) + "</DD>\n"; | ||
| } | ||
|
|
||
| public String toString(Tag[] arg0) { | ||
| String string = ""; | ||
| if (arg0 != null && arg0.length != 0) { | ||
| string = "\n<DT><b>Usage:</b><DD>"; | ||
|
|
||
| for (int i = 0; i < arg0.length - 1; ++i) { | ||
| string = string + XSLUsage.getHTML(arg0[i]) + ", "; | ||
| } | ||
|
|
||
| string = string + XSLUsage.getHTML(arg0[arg0.length - 1]); | ||
| string = string + "</DD>\n"; | ||
| return string; | ||
| } else { | ||
| return string; | ||
| } | ||
| @Override | ||
| public String toString(Tag[] tags) { | ||
| if (tags == null || tags.length == 0) | ||
| return ""; | ||
|
|
||
| String string = "\n<DT><b>Usage:</b><DD>"; | ||
| for (Tag tag : tags) | ||
| string = string + XSLUsage.getHTML(tag) + ", "; | ||
|
|
||
| // Remove trailing ", ", add end tag | ||
| return string.substring(0, string.length() - 2) + "</DD>\n"; | ||
| } | ||
|
|
||
| public static void register(Map tagletMap) { | ||
| XSLUsageTag tag = new XSLUsageTag(); | ||
| Taglet t = (Taglet) tagletMap.get(tag.getName()); | ||
| if (t != null) { | ||
| if (t != null) | ||
| tagletMap.remove(tag.getName()); | ||
| } | ||
|
|
||
| tagletMap.put(tag.getName(), tag); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Even though adding two profiles is not much code, I suggest just requiring Java 17 for build, and use
--release 8,--target 8to target Java 8 bytecode. It would not require profiles, it would be easier to understand.It would be way easier to support, and it would be way fewer bugs since javac 17 should be tested way better than javac 8.
Uh oh!
There was an error while loading. Please reload this page.
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 had discussed that with @kubycsolutions in #120 (comment).
I said:
To which Joseph replied:
As a first-time contributor to this project, I complied, which is the reason why the solution looks like this and no different. Vladimir @vlsi, can you please discuss with Joseph? My suggestion is to keep the build on JDK 8 for now, i.e. accept my solution, because all the work is done already, and I would be quite disappointed to have invested my time just to revert most of it again now.
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.
My take is that dropping JDK8 build compatibility is a separate discussion, worth considering but a bit of a distraction at this moment.
Changing the Java 8 compilation requirement -- if I'm not hallucinating that the policy still exists; remember I'm just getting back to Xalan 15 years later and still catching up -- is something I'd want to warn our users of. If nothing else, supporting both gives us a deprecation window.
So my take is: Valid question, open it separately as technical debt, discuss, and deal with it as its own issue rather than here.
Other Committers may have other opinions. But I'm going to accept this into my branch; it can always be simplified after that discussion, or folks can throw rocks at it before merge into main if they really feel we need to delay the already-large mvn proposal further.
(FWIW, I'm inclined to withdraw my PR to Master/Main again. It was originally posted for discussion; it wasn't ever ready for merge and in hindsight I should have just asked folks to review it unsquashed on the fork. And in fact once we have it ready to consider merging, I'm tempted to reconstruct it in proper incremental change form, thus undoing the squash and making the actual concepts clearer in the history, at the admitted cost of losing some history of my mistakes. Downside is my being distracted by it that much longer. I freely admit I mis-sized the effort to learn the Maven Way.)
Uh oh!
There was an error while loading. Please reload this page.
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.
Agreed.
If it still exists, it is a bit weird. Java 8 is no longer supported, even as an LTS release. I understand that Xalan should run on Java 8, but where is the value in building it on Java 8? Anyway, I implemented it that way and hope you can merge it soon. Sorry for the pressure, but I always try to minimise the difference between touch time and cycle time in everything I do. Frequent context changes lead to loss of efficiency. Moreover, code rots.
True.
Simply mark it as a draft PR.
That would be a total waste of time. It's what it is. (Sorry, I watched "The Irishman", could not resist.) That PR was open too long already. I suggest focusing on getting it done, reviewed and merged. It does not need to be perfect, just good enough. Besides, perfection would be in stark contrast to the rest of the code in Xalan-J. 😉 IMO, the sooner we can leave Ant behind, the better.
Moreover, I would not like to see my commits being redone by someone else who is then listed as an author.