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

fix #63176: note input cursor glitches in voice 2 #2033

Closed
wants to merge 1 commit into from

Conversation

MarcSabatella
Copy link
Contributor

This fixes both issues listed in the original report.

Bug A is handled with the code involving "keepSelected". Basically, I detect the case where there is no cr at input position, so I fall back to the selected note, but also set a flag saying not to then go back one step from there. It is conceivable some case might exist where this prevents the cursor from moving backwards in places where it actually should, but I can't actually make this happen. I was especially concerned about navigation in the presence of grace notes, but so far so good.

Bug B is fixed with code to explicit select the cr at the input position if you try to move to the next chord but there is no next chord. Otherwise, the input cursor was being advanced and we were losing any chance of finding the current chord.

One kind of bothersome thing I realized while looking at this code: we are spending quite a bit of effort managing the input cursor via moveInputPos() for each command, when really, it seems the act of selecting an element does this for us in many cases. Sometimes we call moveInputPos() with one position, then end up selecting some other element and the original moveInput() gets clobbered. It actually does the right thing in the end - I think we could probably eliminate some of the moveInputPos() calls. But this code has been very delicate over the years so I didn't make any substantive changes.

@lasconic
Copy link
Contributor

Thank you for the investigation. I fixed it a bit differently in 5c0b817
It should work the same and I feel it's a bit clearer.

Regarding B, it's bad that we don't reselect the rest when we deleted the chord. I wrongly commited this 4507498 and then reverted it because it has nothing to do with this fix. In any case, I would like to remove this deselectAll() in deleteItem() because sometimes it would make sense to select something after deletion.

@lasconic lasconic closed this Jul 12, 2015
@MarcSabatella
Copy link
Contributor Author

Yeah, there have definitely been user requests that deleting something leave something else selected. I'm not sure I agree with this in the general case. Usually, pressing Delete a second time after deleting something has no effect in most programs other than things like word processors. But having it select the rest specifically after deleting a note does make sense to me. Or, for that matter, selecting the next note, which would make delete of notes work more like a word processor.

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