Skip to content

Add key action support in multi-edit mode - #48

Closed
RibosomeK wants to merge 1 commit into
sdercolin:mainfrom
RibosomeK:add-KeyActions-support-in-multiple-entry-edit-mode
Closed

RibosomeK wants to merge 1 commit into
sdercolin:mainfrom
RibosomeK:add-KeyActions-support-in-multiple-entry-edit-mode

Conversation

@RibosomeK

Copy link
Copy Markdown
Contributor

Hi, in this PR, I add two feature for multi-edit mode, as well as its chinese translation.
First is SetValue1-2 key actions support. Under multi-edit mode, value 1 is the first parameter on the left of the cursor, value 2 is the same but on the right.
The second one is two extra key actions for setting left and right parameter of current entry. But unfortunately, there is bug in getting newestcurrentIndex value, and I dont't know how to fix that. Can you help me on solving this problem?

…yAction to set currentEntry left and right border, along with chinese translation
@sdercolin

sdercolin commented Nov 6, 2024 •

Copy link
Copy Markdown
Owner

@RibosomeK sorry for the late response.
I rechecked our previous conversation and found some differences with your implementation here, so let me summarize my understanding here.

We are going to do 2 things here.

  1. Add a feature for multiple edit mode, to move the nearest lines (on left/right side) to the current cursor position. This should not depend on the current entry index, instead, only depends on the cursor position. Let me call it Snap Closest Left Parameter to Cursor (in Chinese maybe 吸附距离光标最近的左侧参数线). You are naming it SetCurrentEntryLeft, but it should not be the "current entry".
  • Actually, it's also useful for single edit mode, because we also have several lines on the screen at the same time
  1. Simply support setValueX for multiple edit mode and keep all existing behaviors. When selecting the target line to move, this feature doesn't depend on the cursor position, instead, depends on the "current entry index" and the "shortcutIndex" defined in the labeler configs. (e.g. nnsvs-singer-labeler already has them set)

Please check the following template.
I refactored some existing code and separate the two features we are working on, hope it would be more readable.
If you think it's okay, please update your impl using it. After that, let's talk about the detailed implementation, since it's quite complicated.

fun getUpdatedEntriesByKeyAction(
    action: KeyAction,
    appConf: AppConf,
    labelerConf: LabelerConf,
): Pair<List<EntryInPixel>, Int>? {
    val paramIndex = when (action) {
        KeyAction.SetValue1 -> 0
        KeyAction.SetValue2 -> 1
        KeyAction.SetValue3 -> 2
        KeyAction.SetValue4 -> 3
        KeyAction.SetValue5 -> 4
        KeyAction.SetValue6 -> 5
        KeyAction.SetValue7 -> 6
        KeyAction.SetValue8 -> 7
        KeyAction.SetValue9 -> 8
        KeyAction.SetValue10 -> 9
        else -> null
    }
    if (paramIndex != null) {
        return getUpdatedEntriesBySetParameterKeyAction(paramIndex, appConf, labelerConf)
    }

    val isCursorSnapRightSide = when (action) {
        KeyAction.SnapClosestLeftParameter -> false
        KeyAction.SnapClosestRightParameter -> true
        else -> null
    }
    if (isCursorAction) {
        return getUpdatedEntriesByCursorSnapKeyAction(isCursorSnapRightSide, appConf, labelerConf)
    }
    return null
}

private fun getUpdatedEntriesBySetParameterKeyAction(
    paramIndex: Int,
    appConf: AppConf,
    labelerConf: LabelerConf,
): Pair<List<EntryInPixel>, Int>? {

    // remove this -> Only used in single edit mode
    // remove this -> if (entries.size != 1) return null

    val fieldCount = this.labelerConf.fields.filter { it.shortcutIndex != null }.size
    val pointIndex = when {
        // TODO: Update the point index supporting multiple entries
        paramIndex == 0 -> MarkerCursorState.START_POINT_INDEX
        paramIndex == fieldCount + 1 -> MarkerCursorState.END_POINT_INDEX
        paramIndex <= fieldCount -> this.labelerConf.fields.indexOfFirst { it.shortcutIndex == paramIndex }
            .takeIf { it >= 0 } ?: return null
        else -> return null
    }
    val cursorPosition = cursorState.value.position ?: return null
    val lockDrag = appConf.editor.lockedSettingParameterWithCursor &&
        when (appConf.editor.lockedDrag) {
            AppConf.Editor.LockedDrag.UseLabeler -> {
                val lockedDragByBaseField =
                    labelerConf.lockedDrag.useDragBase &&
                        labelerConf.fields.getOrNull(pointIndex)?.dragBase == true
                val lockedDragByStart =
                    labelerConf.lockedDrag.useStart && pointIndex == MarkerCursorState.START_POINT_INDEX
                lockedDragByBaseField || lockedDragByStart
            }
            AppConf.Editor.LockedDrag.UseStart -> pointIndex == MarkerCursorState.START_POINT_INDEX
            else -> false
        }
    val entries = if (lockDrag) {
        getLockedDraggedEntries(pointIndex, cursorPosition, forcedDrag = false)
        // TODO: for multiple mode, we might need to return all `pointIndex`es that are actually affected by the locked drag
        // instead of the current `pointIndex`. so we may need to change the function signature, and update corresponding usages of the return value in `fun MarkerState.editEntryIfNeeded` in `Marker.kt`
    } else {
        getDraggedEntries(pointIndex, cursorPosition, forcedDrag = false)
    }
    return entries to pointIndex
}

private fun getUpdatedEntriesByCursorSnapKeyAction(
    isRightSide: Boolean,
    appConf: AppConf,
    labelerConf: LabelerConf,
): Pair<List<EntryInPixel>, Int>? {
    // TODO: implement this
    // can be similar to `getUpdatedEntriesBySetParameterKeyAction` but
    // 1. `pointIndex` is decided by the `cursorPosition`
    // 2. `lockDrag` is always `false`, because we don't want to move other parameters at the same time here. This is to say, `appConf.editor.lockedSettingParameterWithCursor` should not affect this new feature.
}

sdercolin added a commit that referenced this pull request Jul 10, 2026
Reworks PR #48 on the current dev branch.

In multiple entry edit mode:
- SetValue1 / SetValue2 now set the left / right border of the entry
  under the cursor to the cursor position.
- Adds SetCurrentEntryLeft / SetCurrentEntryRight actions that set the
  left / right border of the current entry regardless of cursor position.

The original PR plumbed a MutableState<Int> for the current index and
hand-rolled the flattened point-index math (which was buggy). The dev
branch already exposes `project` on MarkerState and centralizes the
flattened-index scheme, so this reads the current index from
`project.currentModule.currentIndex` and reuses the existing
getDraggedEntries / border-index helpers. Single edit mode behavior is
unchanged.

Adds English/Chinese/Japanese/Korean strings and tests covering the new
multi-edit resolution paths.

Co-authored-by: RibosomeK <122797zhyktb@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sdercolin

Copy link
Copy Markdown
Owner

Superseded by #111, which reworks this feature on the current dev branch (the current-index bug is resolved there since MarkerState now exposes the project directly). Thanks @RibosomeK — credited as co-author on the merged commit. Closing this one.

@sdercolin sdercolin closed this Jul 10, 2026
@sdercolin

Copy link
Copy Markdown
Owner

The reworked version of this PR (#111) has been released in 1.7.0-beta2. Thank you for the contribution, @RibosomeK!

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