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

feat: add minimal sync API support #159

Merged
merged 5 commits into from
Apr 29, 2021
Merged

feat: add minimal sync API support #159

merged 5 commits into from
Apr 29, 2021

Conversation

@JounQin
Copy link
Contributor Author

JounQin commented Apr 27, 2021

I'm patching MLRule at https://github.com/rx-ts/markuplint-sync/pull/1/files#diff-1723758355313af6ffa725427997e9f6201b45d62be77ddce8d045585ec01e96, it's a little bit hacky.

If you agree to merge it into this PR, that would be better. If not, I'll just patch as-is.

@YusukeHirao
Copy link
Member

@JounQin

You really need to change just only walk and walkOn even if you fork that, don't you?

@JounQin
Copy link
Contributor Author

JounQin commented Apr 28, 2021

@YusukeHirao

I'm not forking markuplint but extending and patching. (with class X extends Y, and Object.defineProperty(M.prototype, x, {value: yyy}))

First, the current rules must be changed to be sync compatible to run, so I change @markuplint/rules, walker of them.

Second, MLRule.create is used in @markuplint/rules to create rules, so I cannot extend MLRule but patching it, so I expose getter for #v and #f to reuse it in patching.

Last not least, if we support sync rule, the current walk and walkOn is not correct at #157 (comment), so I change it, and it is compatible with current async rules of course(rule-textlint test cases proofs.)

So, is there any unexpected changes here for you?

@JounQin
Copy link
Contributor Author

JounQin commented Apr 28, 2021

@YusukeHirao Any luck to merge and release then? markuplint-sync is ready now.

@JounQin JounQin requested a review from YusukeHirao April 29, 2021 00:04
@YusukeHirao YusukeHirao merged commit c47a79f into markuplint:master Apr 29, 2021
@YusukeHirao
Copy link
Member

@JounQin

Thank you for everything.

First, I will prerelease that added @dev tag.
I want to release the public version after I resolve some issues.

@JounQin JounQin deleted the feat/sync_api branch April 29, 2021 01:58
@JounQin
Copy link
Contributor Author

JounQin commented Apr 29, 2021

First, I will prerelease that added @dev tag.

Thank you, that would be appreciated.

I want to release the public version after I resolve some issues.

Looking forward it.

@YusukeHirao
Copy link
Member

@JounQin

Sorry, I didn't the tagged prerelease.
But I released it as canary.

Successfully published:

  • markuplint@1.7.0-alpha.5+c47a79f
  • @markuplint/file-resolver@1.3.0-alpha.13+c47a79f
  • @markuplint/i18n@1.2.0-alpha.33+c47a79f
  • @markuplint/ml-core@1.5.0-alpha.5+c47a79f
  • @markuplint/rule-textlint@0.1.0-alpha.5+c47a79f
  • @markuplint/rules@1.5.0-alpha.5+c47a79f

@JounQin
Copy link
Contributor Author

JounQin commented Apr 29, 2021

That's OK, it should be same for markuplint-sync for now.

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