-
Notifications
You must be signed in to change notification settings - Fork 81
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
Post life cycle #244
Post life cycle #244
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
mdoc.docs.EvilplotModifier | ||
mdoc.docs.LifeCycleModifier |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package mdoc.docs | ||
|
||
import mdoc._ | ||
import mdoc.internal.cli.{Exit, Settings} | ||
|
||
/** | ||
* Global counter used to test the [[mdoc.Main]] process counting. | ||
*/ | ||
object LifeCycleCounter { | ||
val numberOfStarts: ThreadLocal[Integer] = ThreadLocal.withInitial( () => 0 ) | ||
val numberOfExists: ThreadLocal[Integer] = ThreadLocal.withInitial( () => 0 ) | ||
} | ||
|
||
class LifeCycleModifier extends PostModifier { | ||
val name = "lifecycle" | ||
|
||
// Starts and stops per instance | ||
var numberOfStarts = 0 | ||
var numberOfExists = 0 | ||
// Pre and post processing per instance | ||
var numberOfPreProcess = 0 | ||
var numberOfPostProcess = 0 | ||
|
||
def process(ctx: PostModifierContext): String = { | ||
// Used for checking the counting | ||
s"numberOfStarts = $numberOfStarts ; numberOfExists = $numberOfExists ; numberOfPreProcess = $numberOfPreProcess ; numberOfPostProcess = $numberOfPostProcess" | ||
} | ||
|
||
/** | ||
* This is called once when the [[mdoc.Main]] process starts | ||
* @param settings CLI or API settings used by mdoc | ||
*/ | ||
override def onStart(settings: Settings): Unit = { | ||
numberOfStarts += 1 | ||
LifeCycleCounter.numberOfStarts.set(LifeCycleCounter.numberOfStarts.get() + 1) | ||
} | ||
|
||
override def preProcess(ctx: PostModifierContext): Unit = { numberOfPreProcess += 1} | ||
|
||
override def postProcess(ctx: PostModifierContext): Unit = { numberOfPostProcess += 1 } | ||
|
||
/** | ||
* This is called once when the [[mdoc.Main]] process finsihes | ||
* @param exit is the exit code returned by mdoc's processing | ||
*/ | ||
override def onExit(exit: Exit): Unit = { | ||
numberOfExists += 1 | ||
LifeCycleCounter.numberOfExists.set(LifeCycleCounter.numberOfExists.get() + 1) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,25 @@ | ||
package mdoc | ||
|
||
import java.util.ServiceLoader | ||
import mdoc.internal.cli.Settings | ||
|
||
import mdoc.internal.cli.{Exit, Settings} | ||
import metaconfig.ConfDecoder | ||
import metaconfig.ConfEncoder | ||
import metaconfig.ConfError | ||
import metaconfig.generic.Surface | ||
|
||
import scala.meta.inputs.Input | ||
import scala.meta.io.AbsolutePath | ||
import scala.collection.JavaConverters._ | ||
import scala.meta.io.RelativePath | ||
|
||
trait PostModifier { | ||
val name: String | ||
def onStart(settings: Settings): Unit | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These need to have default implementations for backwards compatibility There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. Will do. Will a simple return of Apologies for the newbie questions. I as planning to make another pull request. Should I or do I keep working here? If I do open a new one, their seem to be a "review pull " option. Should I use that instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
def preProcess(ctx: PostModifierContext): Unit | ||
def process(ctx: PostModifierContext): String | ||
def postProcess(ctx: PostModifierContext): Unit | ||
def onExit(exit: Exit): Unit | ||
} | ||
|
||
object PostModifier { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,4 @@ | ||
tests.markdown.EvilplotPostModifier | ||
tests.markdown.BulletPostModifier | ||
tests.markdown.LifeCycleModifier | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package tests.markdown | ||
|
||
import mdoc._ | ||
import mdoc.internal.cli.{Exit, Settings} | ||
|
||
/** | ||
* Global counter used to test the [[mdoc.Main]] process counting. | ||
*/ | ||
object LifeCycleCounter { | ||
val numberOfStarts: ThreadLocal[Integer] = ThreadLocal.withInitial( () => 0 ) | ||
val numberOfExists: ThreadLocal[Integer] = ThreadLocal.withInitial( () => 0 ) | ||
} | ||
|
||
class LifeCycleModifier extends PostModifier { | ||
val name = "lifecycle" | ||
|
||
// Starts and stops per instance | ||
var numberOfStarts = 0 | ||
var numberOfExists = 0 | ||
// Pre and post processing per instance | ||
var numberOfPreProcess = 0 | ||
var numberOfPostProcess = 0 | ||
|
||
def process(ctx: PostModifierContext): String = { | ||
// Used for checking the counting | ||
s"numberOfStarts = $numberOfStarts ; numberOfExists = $numberOfExists ; numberOfPreProcess = $numberOfPreProcess ; numberOfPostProcess = $numberOfPostProcess" | ||
} | ||
|
||
/** | ||
* This is called once when the [[mdoc.Main]] process starts | ||
* @param settings CLI or API settings used by mdoc | ||
*/ | ||
override def onStart(settings: Settings): Unit = { | ||
numberOfStarts += 1 | ||
LifeCycleCounter.numberOfStarts.set(LifeCycleCounter.numberOfStarts.get() + 1) | ||
} | ||
|
||
override def preProcess(ctx: PostModifierContext): Unit = { numberOfPreProcess += 1} | ||
|
||
override def postProcess(ctx: PostModifierContext): Unit = { numberOfPostProcess += 1 } | ||
|
||
/** | ||
* This is called once when the [[mdoc.Main]] process finsihes | ||
* @param exit is the exit code returned by mdoc's processing | ||
*/ | ||
override def onExit(exit: Exit): Unit = { | ||
numberOfExists += 1 | ||
LifeCycleCounter.numberOfExists.set(LifeCycleCounter.numberOfExists.get() + 1) | ||
} | ||
} |
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.
Settings can’t be exposed in the public api since it’s internal. We could add a public interface on the class if that helps
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.
@olafurpg Apologies for closing this but the CI/CD failed. In particular one tests assumes that the files are processed in a given order. Need to correct this.
Settings are not required so I will remove it.
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 can add
MainSettings
as an argumenthttps://github.com/scalameta/mdoc/blob/cf4aa2872e44a1abef1badbfe0544bf39d1a554c/mdoc/src/main/scala/mdoc/MainSettings.scala
We can add public methods down the road to access fields on the private
Settings
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.
Alternatively, we can add a
abstract class Settings
in the publicmdoc
API and pass that in even if it has no public members for now. It's desirable to pass in one parameter just so we can pass in more information in the future without breaking the public APIThere 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 don't understand the use of
MainSettings
.Settings
does not extend this class. Note that I callonStart
method inMainOps
. Can you explain?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.
Settings is a case class which gets auto generated methods like “copy” that break binary compatibility on removal/addition of fields. This makes case classes essentially unsuitable for public APIs unless you’re certain you will never need to add/remove a field.
MainSettings is just a thin wrapper around the case class that has a stable api that we can evolve in a binary compatible way.
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.
Thanks for the explanation.
MainSettings
is now being used. I have tried to make as few changes as possible. They are basically limited toMainOps
. Not too happy with it because I would have liked to make the calls within theMainOps.run
method. However this would entail changing the parameters of the constructor orrun
(Context
is used in many places and my not be the best change).Please see #245