Skip to content
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

Template overload for .調整 #35

Draft
wants to merge 1 commit into
base: dev-0.14
Choose a base branch
from
Draft

Conversation

graphemecluster
Copy link
Member

No description provided.

@graphemecluster
Copy link
Member Author

Badly written 😕

Copy link
Member

@syimyuzya syimyuzya left a comment

Choose a reason for hiding this comment

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

Three things left and we will be ready to merge this :D

  • Setting 重紐 in an expression does not work currently. (I've added a new test case for it)

  • Variable naming:

    • tokens should be 屬性s (because it's grouped by 屬性)
    • 屬性 should be fragment
    • fragments fragment should both be tokens
  • .調整 allows the value part of a predicate to be a template argument (like .調整`${'見'}母`), while .屬於 only accepts template arguments as whole predicates (.屬於`$'見母'}` but not .屬於`${'見'}母`).

    Shall we solve (or just doc about) this subtle (i.e. easy to overlook) difference between .調整 and .屬於?

src/lib/音韻地位.spec.ts Show resolved Hide resolved
@graphemecluster
Copy link
Member Author

@syimyuzya (Well, first of all, do you think we ever need this? I guess you and Ayaka may find this feature redundant?)

@syimyuzya
Copy link
Member

syimyuzya commented Sep 13, 2022

More on the expression syntax for .屬於:

When I rewrote .屬於, I intentionally made it only accept whole predicates to avoid too much complexity and difficult design choices in its syntax. Examples be like:

  1. Should we accept 重紐${'A'}類? Why or why not? If so, should we also accept ${'重紐A'}類, or 重${'紐A'}類? (:herb:
  2. Should we accept ${() => cond ? '見' : ''}影母? How about 影${() => cond ? '見' : ''}母?
  3. Should we accept 清${() => cond ? '母' : '韻'}?
  4. How should we parse ${() => true}影母? Is it a valid expression (true && 影母), or is it invalid?
  5. Should we accept ${() => '去' : '仄'}聲? (note that 仄聲 is not a basic 音韻屬性) If so, should we also accept 陽${() => cond ? '' : '聲'}韻? (:herb:

😂😂

@syimyuzya
Copy link
Member

@syimyuzya (Well, first of all, do you think we ever need this? I guess you and Ayaka may find this feature redundant?)

I think it's not redundant at least (except for the template part, maybe that's overkill 🙈). Not sure how much it's need either, though.

.調整 is currently the only method where users have to write like { 聲: '去' } but not the more natural '去聲' (other methods have, for example, from描述 instead of the constructor, 屬於 instead of === or [...].includes), this PR just fills the gap.

@syimyuzya
Copy link
Member

syimyuzya commented Sep 14, 2022

except for the template part, maybe that's overkill

The expression syntax for .調整 is quite restricted and much simpler than .屬於 (in particular, no logical operators, parentheses or sub-expressions), so IMO the ordinary string template (.調整(`${'見'}母`)) should suffice already, and there is not much to benefit from special handling of template arguments.

src/lib/音韻地位.ts Outdated Show resolved Hide resolved
Fix unrecognized `重紐A類`
@graphemecluster
Copy link
Member Author

IMO:

  1. Should we accept 重紐${'A'}類? Why or why not? If so, should we also accept ${'重紐A'}類, or 重${'紐A'}類? (🌿

Accept 重紐${'A'}類 but not ${'重紐A'}類 or 重${'紐A'}類.

  1. Should we accept ${() => cond ? '見' : ''}影母? How about 影${() => cond ? '見' : ''}母?

I would probably only accept ${() => cond ? '見影' : '影'}母 if you didn't mention them, but it seems to be reasonable to accept them.

  1. Should we accept 清${() => cond ? '母' : '韻'}?

Yes for me. At least, the current implementation of .調整 accepts 清${cond ? '母' : '韻'}.

  1. How should we parse ${() => true}影母? Is it a valid expression (true && 影母), or is it invalid?

Should be OK to keep it valid.

  1. Should we accept ${() => '去' : '仄'}聲? (note that 仄聲 is not a basic 音韻屬性) If so, should we also accept 陽${() => cond ? '' : '聲'}韻? (🌿

I am neutral to ${() => cond ? '去' : '仄'}聲 (in fact it seems that I made it invalid for simplicity in the past 😂😂), but 陽${() => cond ? '' : '聲'}韻 should definitely be rejected.

@graphemecluster
Copy link
Member Author

And please complete the documentation for me, I am too lazy, please forgive me, @syimyuzya

@syimyuzya
Copy link
Member

syimyuzya commented Sep 15, 2022

3. Should we accept ${() => cond ? '見' : ''}影母? How about 影${() => cond ? '見' : ''}母?

I would probably only accept ${() => cond ? '見影' : '影'}母 if you didn't mention them, but it seems to be reasonable to accept them.

7. How should we parse ${() => true}影母? Is it a valid expression (true && 影母), or is it invalid?

Should be OK to keep it valid.

Note the syntactic ambiguity here: How should we then handle 來母 或 ${() => cond ? '見' : true }影組? 😂

Be aware that:

  1. The type of a lazy template argument cannot be predicted until it gets evaluated;
  2. We can't just let lazy arguments affect the (already parsed) syntax tree, even in this simple case; otherwise it would bring more surprises.

@syimyuzya
Copy link
Member

主開發分支的 92ab876 已經加入了支援普通字串的 .調整

不過這個 PR 可以保持 open,用來繼續討論 tagged literal 版本的設計細節w

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