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

Make jianpu-ly importable #15

Closed
SilverRainZ opened this issue Jul 19, 2022 · 11 comments
Closed

Make jianpu-ly importable #15

SilverRainZ opened this issue Jul 19, 2022 · 11 comments

Comments

@SilverRainZ
Copy link

SilverRainZ commented Jul 19, 2022

Hi, I am the author of sphinxnotes-lilypond, an extension for embedding LilyPond score into Sphinx documentation. I want to add Jian Pu support (sphinx-notes/lilypond#17) for it.

It seems that this project is a CLI tool for now. It would be better to make this project importable, so other project (like sphinxnotes-lilypond) can use it.

Thank you~

@SilverRainZ SilverRainZ changed the title Provide pypi package and a converter function Make jianpu-ly importable Jul 19, 2022
@SilverRainZ
Copy link
Author

I am working on this and will file a PR soon. hope it can be merged eventually.

谢谢~

@ssb22
Copy link
Owner

ssb22 commented Jul 19, 2022

哎哟 how much have you done so far? I started moving some things around already .. basically we first need to make sure it can be imported and doesn't try to do things with sys.argv when __name__ is not "__main__"

@SilverRainZ
Copy link
Author

I just make it work in an ugly way, it is quite complex so I indented all functions one level except help and IO.

You are the one most familiar with this program, if you already started, I am so glad to see that, you can do it better than me. I just 乐享其成 :D

ssb22 added a commit that referenced this issue Jul 19, 2022
@ssb22
Copy link
Owner

ssb22 commented Jul 19, 2022

Just indenting all functions might not work so well: some things needed to be declared global so they could be read by other functions later on. Try now (see test.py). But I'm not sure if it's ready for PyPI .. I expect they have stricter style requirements (haven't checked)

@SilverRainZ
Copy link
Author

It works well. Thank you very much~

PyPI is not necessary, I can include this as a git submodule.

@SilverRainZ
Copy link
Author

Hi @ssb22, the module name contains a dash so we can't use the import statement directly, any idea to improve this?

https://stackoverflow.com/questions/8350853/how-to-import-module-when-module-name-has-a-dash-or-hyphen-in-it

@ssb22
Copy link
Owner

ssb22 commented Jul 20, 2022

Please just use jianpuly = __import__("jianpu-ly") for now. I know having a hyphen in it is not ideal, but I'm not sure I can change that now without breaking everybody's scripts, except perhaps by adding a second file as a wapper, but then it will no longer be all in one file for those who just wget it.

@SilverRainZ
Copy link
Author

Ok, it is ok, I renamed it by using s symlink: https://github.com/sphinx-notes/lilypond/tree/jianpu/sphinxnotes/lilypond

@ssb22
Copy link
Owner

ssb22 commented Jul 20, 2022

好主意☺

@SilverRainZ
Copy link
Author

Since there is no other problem, I close this issue.

Thank you very much :D

@SilverRainZ
Copy link
Author

Finally, we can write Jian Pu in Sphinx: https://sphinx.silverrainz.me/lilypond/#jianpu-numbered-musical-notation :D

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

No branches or pull requests

2 participants