Skip to content

Conversation

kitbellew
Copy link
Collaborator

In InternalTree, use it to tokenize, before resorting to syntax parsing. Also, if that Input contained any TokenizerOptions, they would naturally be used. Follow-on to #3786.

Copy link
Member

@bjaglin bjaglin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM to favor the options from the potential Input (the same should probably be done in textAsInput), but I question the implicit captures as a fallback (I guess that the intent?) to get options if we can't find the Input.


private[meta] def retokenizeFor(
dialect: Dialect
)(implicit tokenize: Tokenize, tokenizerOptions: Option[TokenizerOptions]): Tokens = this match {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
)(implicit tokenize: Tokenize, tokenizerOptions: Option[TokenizerOptions]): Tokens = this match {
)(implicit tokenize: Tokenize): Tokens = this match {


private[meta] def retokenize(implicit
tokenize: Tokenize,
dialect: Dialect,
tokenizerOptions: Option[TokenizerOptions]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tokenizerOptions: Option[TokenizerOptions]

case Lit.String(value) =>
val input = Input.VirtualFile("<InternalTrees.tokens>", value)
Tokens(Array(Constant.String(input, dialect, 0, value.length, value)))
case _ => implicitly[Tokenize].apply(textAsInput, dialect).get
case _ => tokenize(origin.inputOpt.getOrElse(textAsInput), dialect).get
}

private[meta] def textAsInput(implicit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not visible in the diff, but another implicit Option[TokenizerOptions] is captured - it's probably more correct to get it from the input just like above?

In InternalTree, use it to tokenize, before resorting to syntax parsing.
Also, if that Input contained any TokenizerOptions, they would naturally
be used.
@kitbellew kitbellew merged commit 1c1bcee into scalameta:main Aug 25, 2024
24 checks passed
@kitbellew kitbellew deleted the 3786_15 branch August 25, 2024 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants