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

Adding a "replace mode" to MIDI Input V2.x #1039

Closed
wants to merge 8 commits into from

Conversation

deviskra
Copy link
Contributor

Adding a "replace mode" to MIDI Input #866

Adding a "replace mode" to MIDI Input frescobaldi#866
@deviskra deviskra changed the title V2.x Adding a "replace mode" to MIDI Input V2.x Nov 19, 2017
@uliska
Copy link
Collaborator

uliska commented Nov 20, 2017

Thanks for the contribution.
From your description on #866 this looks very interesting. Unfortunately I haven't set up MIDI input and won't be able to do so.

In order to reach more potential testers you should write a note to the Frescobaldi and lilypond-user mailing lists, pointing both to the original feature request and this pull request.

Not sure how to optimize the regular expression to fit lilypond note names exclusively. Now every 3 or less letters word is a note name if it doesn't start with #, \, _, -, ^ symbols and doesn't end with dot. Of course it also finds such 'notes' within quotes if happen. I am not experienced in Python nor in regular expressions so I give up to make it better for now. From the other side it's pretty usable even so.
@fedelibre
Copy link
Member

This fixes issue #839

I may be interested to test it if it was for Frescobaldi 3. There's any reason why this was implemented for v2 only?

@deviskra
Copy link
Contributor Author

deviskra commented Jan 30, 2018

I have had problems with qt libraries trying to launch v3, but v2 works fine for me, so I typed my code where it works. It's very simple bit of code and it wouldn't be a problem to paste it to another version, I believe. Are there any functional advantages of the v3, by the way? I'm using v2 and pretty satisfied.

@uliska
Copy link
Collaborator

uliska commented Jan 30, 2018

Are there any functional advantages of the v3, by the way? I'm using v2 and pretty satisfied.

https://github.com/wbsoft/frescobaldi/blob/master/ChangeLog

Not that many, yet. But you'll be further and further away from the current state. For example we're still waiting for (me to) merging the new version control functionality from last year's GSoC, and there is definitely more to come ...

The instructions for running Frescobaldi 3 (on Linux) have been greatly improved BTW: https://github.com/wbsoft/frescobaldi/wiki/Installation-on-Linux-(General)

@fedelibre
Copy link
Member

@deviskra I've tested your patch successfully. It's a very nice addition!

However, to get it merged to master, it should be rebased on master.
I've just tried it. I've squashed your commits into one, tried to solve all the conflicts and changed an import from PyQt4 to PyQt5. The new option appears but MIDI capture doesn't work anymore. I guess I did something wrong when solving merging conflicts; or your code should be updated to work with master. I don't know, as I've only tried to learn programming a few years ago...

I've pushed my rebase here, if you want to test it.

@deviskra
Copy link
Contributor Author

@fedelibre Thank you for testing and appreciation of my scribbles. I tried once to install higher versions of Python on my machine, but something went wrong, so I haven't tried Frescobaldi from the master branch yet. Though, I haven't tried hard cause 2.x version works fine for me and I am lazy. Maybe it's time to try again.

@fedelibre
Copy link
Member

@deviskra I take back my previous comment. My midi-replace-mode-rebased branch is working correctly!

I just made a stupid mistake. I had the Re-pitch mode flagged while the cursor was on an empty line: obviously there was nothing to be replaced and nothing happened.

@uliska
Copy link
Collaborator

uliska commented Sep 24, 2018

@fedelibre can I understand your last comment that this patch works as expected?

@uliska
Copy link
Collaborator

uliska commented Sep 24, 2018

I mean: is this PR ready to be merged? And does it close #839 and #866?

@fedelibre
Copy link
Member

Yes, it works as expected.

#839 is actually a sort of "metaissue" as it aims for two features already covered by other issues:

Up to you if closing #839 or not.

@antanasb
Copy link

antanasb commented May 7, 2019

Very useful feature if a melody is doubled in other voices! As I understand there are no obstacles to merge it to the v.3 master branch. Why it still isn't?

@fedelibre
Copy link
Member

@antanasb I've now created a PR containing the py3 code which can be merged in master.

@uliska I know you don't have a MIDI keyboard to test it. I believe PR #1152 can be merged, so we'll get feedback more easily from other users.

@uliska
Copy link
Collaborator

uliska commented May 7, 2019

@fedelibre Indeed I don't have a MIDI keyboard available. I would follow your advice without testing myself, provided @wbsoft is OK with that. But I'd ask you (or @deviskra) to re-commit and push with a meaningful commit message that will also give us information outside of Github (i.e. without the reference to an issue.

@fedelibre
Copy link
Member

I'm closing this PR as it's been replaced by the other one.

@fedelibre fedelibre closed this May 8, 2019
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.

None yet

4 participants