Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
michael-o commented on code in PR #180:
URL: https://github.com/apache/maven-doxia/pull/180#discussion_r1428911493
##
doxia-core/src/main/java/org/apache/maven/doxia/index/IndexEntry.java:
##
@@ -304,7 +310,7 @@ public String toString(int depth) {
message.append(", title: ").append(title);
}
-message.append(EOL);
+message.append(System.lineSeparator());
Review Comment:
Attention, this is a semantical change. All others use the idiom above and
there are tests which change the separator with try/finally. This one is caches
upon first call. I wouldn't change it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
michael-o commented on code in PR #180:
URL: https://github.com/apache/maven-doxia/pull/180#discussion_r1428900306
##
doxia-core/src/main/java/org/apache/maven/doxia/index/IndexingSink.java:
##
@@ -104,15 +106,21 @@ private IndexingSink(IndexEntry rootEntry, Sink delegate)
{
stack.push(rootEntry);
usedIds = new HashMap<>();
usedIds.put(rootEntry.getId(), new AtomicInteger());
-init();
+this.type = 0;
+this.title = null;
}
/**
* This should only be called once the sink is closed.
* Before that the tree might not be complete.
* @return the tree of entries starting from the root
+ * @throws IllegalStateException in case the sink was not closed yet
*/
public IndexEntry getRootEntry() {
+if (!isComplete) {
+throw new IllegalStateException(
+"The sink has not been closed yet, i.e. the index tree is
not complete yet");
Review Comment:
This sink...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
michael-o commented on code in PR #180:
URL: https://github.com/apache/maven-doxia/pull/180#discussion_r1428795972
##
doxia-core/src/main/java/org/apache/maven/doxia/parser/AbstractParser.java:
##
@@ -189,6 +229,25 @@ protected boolean isSecondParsing() {
return secondParsing;
}
+@Override
+public void registerSinkWrapperFactory(SinkWrapperFactory factory) {
Review Comment:
Maybe rather `get`/`add` semantic names?
##
doxia-core/src/main/java/org/apache/maven/doxia/index/IndexingSink.java:
##
@@ -76,19 +80,41 @@ public class IndexingSink extends SinkAdapter {
*/
private final Map usedIds;
+private final IndexEntry rootEntry;
+
+/**
+ * @deprecated legacy constructor, use {@link #IndexingSink(Sink)} with
{@link SinkAdapter} as argument and call {@link #getRootEntry()} to retrieve
the index tree afterwards.
+ */
+@Deprecated
+public IndexingSink(IndexEntry rootEntry) {
+this(rootEntry, new SinkAdapter());
+}
+
+public IndexingSink(Sink delegate) {
+this(new IndexEntry("index"), delegate);
+}
+
/**
* Default constructor.
- *
- * @param sectionEntry The first index entry.
*/
-public IndexingSink(IndexEntry sectionEntry) {
+private IndexingSink(IndexEntry rootEntry, Sink delegate) {
+super(delegate);
+this.rootEntry = rootEntry;
stack = new Stack<>();
-stack.push(sectionEntry);
+stack.push(rootEntry);
usedIds = new HashMap<>();
-usedIds.put(sectionEntry.getId(), new AtomicInteger());
+usedIds.put(rootEntry.getId(), new AtomicInteger());
init();
}
+/**
+ * This should only be called once the sink is closed.
+ * Before that the tree might not be complete.
+ * @return the tree of entries starting from the root
+ */
+public IndexEntry getRootEntry() {
+return rootEntry;
+}
Review Comment:
You see a way to throw ISE if the documented condition is not met?
##
doxia-core/src/main/java/org/apache/maven/doxia/parser/SinkWrapperFactory.java:
##
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.doxia.parser;
+
+import org.apache.maven.doxia.sink.Sink;
+
+/**
+ * A factory for a sink wrapping another sink.
+ * The delegate may either be the original sink or another wrapper.
+ * @since 2.0.0
+ */
+public interface SinkWrapperFactory {
+
+/**
+ * By default all wrappers just delegate each method to the wrapped sink's
method.
+ * For certain Sink methods the wrapper may modify the sink before/after
or instead of calling the delegate's method.
+ * @param sink the delegate
+ * @return a new sink wrapping the given one
+ */
+Sink createWrapper(Sink sink);
+
+/**
+ * Determines the order of sink wrappers. The highest ranked wrapper
factory is called first.
+ * @return the rank of this factory
+ */
+int getRank();
Review Comment:
In DI we use the term priority. Maybe that be suit here too? This making the
terms consistent while `Integer.MAX_VALUE` would be the highest one?
Comment talks about order, but method name is rank...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
michael-o commented on code in PR #180:
URL: https://github.com/apache/maven-doxia/pull/180#discussion_r1428794860
##
doxia-core/src/main/java/org/apache/maven/doxia/index/IndexEntry.java:
##
@@ -105,6 +110,10 @@ protected void setId(String id) {
this.id = id;
}
+public boolean hasAnchor() {
+return hasAnchor;
+}
+
/**
* Returns the title.
Review Comment:
Am I stupid, where is the setter?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
michael-o commented on code in PR #180:
URL: https://github.com/apache/maven-doxia/pull/180#discussion_r1428794572
##
doxia-core/src/main/java/org/apache/maven/doxia/parser/Parser.java:
##
@@ -83,4 +85,34 @@ public interface Parser {
* @return true (default value) if comment Doxia events are
emitted
*/
boolean isEmitComments();
+
+/**
+ * Registers a given {@link SinkWrapperFactory} with the parser used in
subsequent calls of {@code parse(...)}
+ * @param factory the factory to create the sink wrapper
+ * @since 2.0.0
+ */
+void registerSinkWrapperFactory(SinkWrapperFactory factory);
+
+/**
+ *
+ * @return all sink wrapper factories in the correct order (both
registered automatically and manually)
+ * @since 2.0.0
+ */
+Collection getSinkWrapperFactories();
+
+/**
+ * Determines whether to automatically generate anchors for each section
found by {@link IndexingSink} or not.
+ * By default no anchors are generated.
+ *
+ * @param emitAnchors {@code true} to emit anchors otherwise {@code false}
(the default)
+ * @since 2.0.0
+ */
+void setEmitAnchors(boolean emitAnchors);
Review Comment:
You mean everything would could potentially have a number group? Then maybe
`setEmitAnchorsForIndexableEntries()` and docs will carry only the information
that it applies to section titles for the moment?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
kwin commented on code in PR #180:
URL: https://github.com/apache/maven-doxia/pull/180#discussion_r1428394870
##
doxia-core/src/main/java/org/apache/maven/doxia/parser/Parser.java:
##
@@ -83,4 +85,34 @@ public interface Parser {
* @return true (default value) if comment Doxia events are
emitted
*/
boolean isEmitComments();
+
+/**
+ * Registers a given {@link SinkWrapperFactory} with the parser used in
subsequent calls of {@code parse(...)}
+ * @param factory the factory to create the sink wrapper
+ * @since 2.0.0
+ */
+void registerSinkWrapperFactory(SinkWrapperFactory factory);
+
+/**
+ *
+ * @return all sink wrapper factories in the correct order (both
registered automatically and manually)
+ * @since 2.0.0
+ */
+Collection getSinkWrapperFactories();
+
+/**
+ * Determines whether to automatically generate anchors for each section
found by {@link IndexingSink} or not.
+ * By default no anchors are generated.
+ *
+ * @param emitAnchors {@code true} to emit anchors otherwise {@code false}
(the default)
+ * @since 2.0.0
+ */
+void setEmitAnchors(boolean emitAnchors);
Review Comment:
Maybe `setEmitAnchorsForIndexEntries(...)`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
kwin commented on code in PR #180:
URL: https://github.com/apache/maven-doxia/pull/180#discussion_r1428392892
##
doxia-core/src/main/java/org/apache/maven/doxia/parser/Parser.java:
##
@@ -83,4 +85,34 @@ public interface Parser {
* @return true (default value) if comment Doxia events are
emitted
*/
boolean isEmitComments();
+
+/**
+ * Registers a given {@link SinkWrapperFactory} with the parser used in
subsequent calls of {@code parse(...)}
+ * @param factory the factory to create the sink wrapper
+ * @since 2.0.0
+ */
+void registerSinkWrapperFactory(SinkWrapperFactory factory);
+
+/**
+ *
+ * @return all sink wrapper factories in the correct order (both
registered automatically and manually)
+ * @since 2.0.0
+ */
+Collection getSinkWrapperFactories();
+
+/**
+ * Determines whether to automatically generate anchors for each section
found by {@link IndexingSink} or not.
+ * By default no anchors are generated.
+ *
+ * @param emitAnchors {@code true} to emit anchors otherwise {@code false}
(the default)
+ * @since 2.0.0
+ */
+void setEmitAnchors(boolean emitAnchors);
Review Comment:
It is potentially for all index entries (although IndexingSink currently
only generates ones for sections, but there are some todos for figures, tables,
...). For sure it is not specific to to ones having a title
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
michael-o commented on code in PR #180:
URL: https://github.com/apache/maven-doxia/pull/180#discussion_r1428238514
##
doxia-core/src/main/java/org/apache/maven/doxia/parser/Parser.java:
##
@@ -83,4 +85,34 @@ public interface Parser {
* @return true (default value) if comment Doxia events are
emitted
*/
boolean isEmitComments();
+
+/**
+ * Registers a given {@link SinkWrapperFactory} with the parser used in
subsequent calls of {@code parse(...)}
+ * @param factory the factory to create the sink wrapper
+ * @since 2.0.0
+ */
+void registerSinkWrapperFactory(SinkWrapperFactory factory);
+
+/**
+ *
+ * @return all sink wrapper factories in the correct order (both
registered automatically and manually)
+ * @since 2.0.0
+ */
+Collection getSinkWrapperFactories();
+
+/**
+ * Determines whether to automatically generate anchors for each section
found by {@link IndexingSink} or not.
+ * By default no anchors are generated.
+ *
+ * @param emitAnchors {@code true} to emit anchors otherwise {@code false}
(the default)
+ * @since 2.0.0
+ */
+void setEmitAnchors(boolean emitAnchors);
Review Comment:
This should be more specific: `setEmitSectionTitleAnchors`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
michael-o commented on PR #180: URL: https://github.com/apache/maven-doxia/pull/180#issuecomment-1837574750 > > I don't see need to modify/intercept the sink at all, a parser can do this perfectly with some buffering. TOCMacro does this buffering with a fresh parser as well. > > The point is that a sink wrapper can be used with all parsers, while buffering on parser level has to be implemented per parser. Are you certain about this? Can't this be done in the `AbstractParser`? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
kwin commented on PR #180: URL: https://github.com/apache/maven-doxia/pull/180#issuecomment-1837439220 > I don't see need to modify/intercept the sink at all, a parser can do this perfectly with some buffering. TOCMacro does this buffering with a fresh parser as well. The point is that a sink wrapper can be used with all parsers, while buffering on parser level has to be implemented per parser. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
michael-o commented on PR #180:
URL: https://github.com/apache/maven-doxia/pull/180#issuecomment-1837260464
> > This seems like a reimplementation of RandomAccessSink: The
RandomAccessSink provides the ability to create a {https://github.com/link
Sink} with hooks. Why not evaluate it if it there.
>
> I was not aware of this class, but `RandomAccessSink` is complex to grasp
for me. I would rather go for reimplementing RandomAccessSink as special
SinkWrapper. AFAIK RandomAccessSink does not allow to intercept sink API calls
and enrich them but just does some buffering and concatenating multiple sink
outputs into one effective one.
I don't see need to modify/intercept the sink at all, a parser can do this
perfectly with some buffering. `TOCMacro` does this buffering with a fresh
parser as well.
> > This should not be related to the TOC macro although the TOC macro will
benefit. A user can request auto anchors regardless of the TOC macro. Don't
conflate them.
>
> For me the TOC is the primary use case, but I am ok with always generating
anchors and TOC macro will just use those.
Correct, TOC will use what has been generated, therefore a parser switch...
> > I think that a solution should be logically identical to
org.apache.maven.doxia.parser.Parser.setEmitComments(boolean). A parser feature
will intercept section title text and buffer until sectionTitle_() is hit,
anchor and title (heading) are emitted.
>
> As long as the default is "emit anchors for every section title" I am fine
with adding an option to disable that. I don't think we should make that an
opt-in behaviour as that would be breaking change from a user's perspective.
Well, making it default would be breaking because Apt, for example does not
have implicit anchors. What I see is tht the opt-in will be enabled from the
Maven Site Plugin invocation and not Doxia directly. That will cover your use
case and won't break anything for others.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
kwin commented on PR #180:
URL: https://github.com/apache/maven-doxia/pull/180#issuecomment-1831556494
> This seems like a reimplementation of RandomAccessSink: The
RandomAccessSink provides the ability to create a {https://github.com/link
Sink} with hooks. Why not evaluate it if it there.
I was not aware of this class, but {{RandomAccessSink}} is complex to grasp
for me. I would rather go for reimplementing RandomAccessSink as special
SinkWrapper. AFAIK RandomAccessSink does not allow to intercept sink API calls
and enrich them but just does some buffering and concatenating multiple sinks
into one effective one.
> This should not be related to the TOC macro although the TOC macro will
benefit. A user can request auto anchors regardless of the TOC macro. Don't
conflate them.
For me the TOC is the primary use case, but I am ok with always generating
anchors and TOC macro will just use those.
> I think that a solution should be logically identical to
org.apache.maven.doxia.parser.Parser.setEmitComments(boolean). A parser feature
will intercept section title text and buffer until sectionTitle_() is hit,
anchor and title (heading) are emitted.
As long as the default is "emit anchors for every section title" I am fine
with adding an option to disable that. I don't think we should make that an
opt-in behaviour as that would be breaking change from a user's perspective.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
michael-o commented on code in PR #180:
URL: https://github.com/apache/maven-doxia/pull/180#discussion_r1408359647
##
doxia-core/src/main/java/org/apache/maven/doxia/macro/MacroRequest.java:
##
@@ -40,15 +40,23 @@ public class MacroRequest {
/** A map of parameters. */
private Map parameters;
+private final Parser currentParser;
/**
* Constructor for MacroRequest.
*
+ * @param currentParser a {@link org.apache.maven.doxia.parser.Parser}
object (which is the parser triggering this macro).
* @param sourceContent a {@link java.lang.String} object.
- * @param parser a {@link org.apache.maven.doxia.parser.AbstractParser}
object.
+ * @param parser a new {@link
org.apache.maven.doxia.parser.AbstractParser} object acting as secondary parser.
* @param param a {@link java.util.Map} object.
* @param basedir a {@link java.io.File} object.
*/
-public MacroRequest(String sourceContent, AbstractParser parser,
Map param, File basedir) {
+public MacroRequest(
+Parser currentParser,
Review Comment:
This is exactly the idea I had, but rejected it because the I consider a
macro something which should not push back, i.e., side effect free.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [DOXIASITETOOLS-320] Auto-generate anchor for TOC entries [maven-doxia]
kwin commented on code in PR #180:
URL: https://github.com/apache/maven-doxia/pull/180#discussion_r1408134053
##
doxia-core/src/main/java/org/apache/maven/doxia/sink/impl/NamesForIndexEntriesSinkWrapper.java:
##
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.doxia.sink.impl;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.maven.doxia.index.IndexEntry;
+import org.apache.maven.doxia.index.SectionLocation;
+import org.apache.maven.doxia.macro.toc.TocMacro;
+import org.apache.maven.doxia.parser.SinkWrapper;
+import org.apache.maven.doxia.sink.Sink;
+import org.apache.maven.doxia.sink.SinkEventAttributes;
+
+/**
+ * Sink wrapper which emits anchors for each section represented by the given
{@link IndexEntry}.
+ * @see TocMacro
+ */
+public class NamesForIndexEntriesSinkWrapper extends SinkWrapper {
+
+private final Map entryByLocation;
+private SectionLocation currentLocation;
+
+public NamesForIndexEntriesSinkWrapper(Sink delegate, IndexEntry index) {
+super(delegate);
+entryByLocation = new HashMap<>();
+populateMap(entryByLocation, index);
+this.currentLocation = new SectionLocation();
+}
+
+private static void populateMap(Map
entryByLocation, IndexEntry entry) {
+entryByLocation.put(entry.getLocation(), entry);
+entry.getChildEntries().forEach(e -> populateMap(entryByLocation, e));
+}
+
+@Override
+public void section(int level, SinkEventAttributes attributes) {
+onStartSection(level);
+super.section(level, attributes);
+}
+
+@Override
+public void section_(int level) {
+super.section_(level);
+onEndSection();
+}
+
+@Override
+public void section1() {
+super.section1();
+onStartSection(1);
+}
+
+@Override
+public void section1_() {
+super.section1_();
+onEndSection();
+}
+
+@Override
+public void section2() {
+super.section2();
+onStartSection(2);
+}
+
+@Override
+public void section2_() {
+super.section2_();
+onEndSection();
+}
+
+@Override
+public void section3() {
+super.section3();
+onStartSection(3);
+}
+
+@Override
+public void section3_() {
+super.section3_();
+onEndSection();
+}
+
+@Override
+public void section4() {
+super.section4();
+onStartSection(4);
+}
+
+@Override
+public void section4_() {
+super.section4_();
+onEndSection();
+}
+
+@Override
+public void section5() {
+super.section5();
+onStartSection(5);
+}
+
+@Override
+public void section5_() {
+super.section5_();
+onEndSection();
+}
+
+private void onStartSection(int level) {
+currentLocation = currentLocation.createChildSection(level);
+IndexEntry entry = entryByLocation.get(currentLocation);
+if (entry != null) {
Review Comment:
Probably a guard similar to
https://github.com/apache/maven-doxia-sitetools/pull/58/files#diff-dabe16ceeba5dc7cb0a15e30dcb7c204a78f4763346f016ab7639ab2f59e78d4L250
needs to be introduced to at least prevent double anchors for APT format.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
