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

update vim-vimlparser #48

Merged
merged 27 commits into from
Dec 13, 2020
Merged

Conversation

itchyny
Copy link
Collaborator

@itchyny itchyny commented Dec 1, 2020

Do not merge, this is still in progress...

I finished the work and the tests finally pass!

@itchyny itchyny marked this pull request as ready for review December 2, 2020 13:40
@itchyny itchyny force-pushed the update-vimlparser-20201201 branch from 6f2f86d to 342a3e0 Compare December 5, 2020 03:36
@itchyny itchyny force-pushed the update-vimlparser-20201201 branch from 342a3e0 to ad5e5b0 Compare December 5, 2020 03:41
@itchyny itchyny force-pushed the update-vimlparser-20201201 branch from 0b547b9 to adf1314 Compare December 6, 2020 08:49
@haya14busa
Copy link
Member

Awesome! Thank you for your work!

Hmm.. the Travis commit integration doesn't work, but the Travis tests are still failing. (Maybe we need to migrate to GitHub Actions...) https://travis-ci.org/github/haya14busa/go-vimlparser/jobs/747912759
Can you fix the travis tests?

@haya14busa
Copy link
Member

Also, I'm not actively maintaining this repository these days.
I sent an invitation to you as a collaborator, so please feel free to accept if you're interested in it.
We can also consider moving the repo under some organizations like vim-jp. What do you think?

@itchyny
Copy link
Collaborator Author

itchyny commented Dec 7, 2020

Hmm.. the Travis commit integration doesn't work, but the Travis tests are still failing. (Maybe we need to migrate to GitHub Actions...) https://travis-ci.org/github/haya14busa/go-vimlparser/jobs/747912759
Can you fix the travis tests?

Done. The Go version was old (1.8) so I updated to 1.14.

@itchyny
Copy link
Collaborator Author

itchyny commented Dec 7, 2020

Also, I'm not actively maintaining this repository these days.
I sent an invitation to you as a collaborator, so please feel free to accept if you're interested in it.
We can also consider moving the repo under some organizations like vim-jp. What do you think?

Moving to vim-jp looks great, @mattn mentioned this topic on Slack. Better to include in vim-jp/vim-vimlparser or different repository? The code base is much different from js and py but maybe we can move into https://github.com/vim-jp/vim-vimlparser/tree/master/go/vimlparser and users can import the package. And running integrated tests is great because some changes has broke compiling to Go code. My concern is the vimlparser.vim in this repository is already diverted from the original due to NODE_PARENEXPR (vim-jp/vim-vimlparser#71).

@haya14busa haya14busa merged commit 46da320 into vim-jp:master Dec 13, 2020
@haya14busa
Copy link
Member

Thank you for your work! 🚀

Yes! I also tried to add the Go change into the original vimlparser repo, but my concerns and problems were...

  1. Backward in-compatible NODE_PARENEXPR change: I tried to make vital vimlparser module to add the backward-incompatible changes but I haven't completed it. Note that the main blocker at that time was just naming if I remember correctly, so it should be not difficult (I just wanted to use "VimL" Parser as module name but some vim-jp members prefer to use a different name and I lost the enthusiasm at that time. Reverting the NODE_PARENEXPR change is another option, but I guess we want this feature ideally?
  2. Maintainance: go-vimlparser changes could hinder the development of vimlparser for vim/Python/JavaScript. I guess you may notice but go-vimlparser contains many ad-hoc code and it's not always easy to follow the original change.

I propose to move the repo to vim-jp org as a separate repo as a first step and we can work on merging the repos later.

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