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: enable texts of search related commnds to show in plural form when mutli files are selected. #1688

Closed
wants to merge 3 commits into from

Conversation

yanCode
Copy link
Contributor

@yanCode yanCode commented Sep 26, 2024

Issue Addressed

Previously, when pressing the c key, the text displayed was always in singular form, even when multiple files were selected:

Copy the file path
Copy the directory path
Copy the filename

Solution

After this fix, when multiple files are selected and the c key is pressed, the nouns are converted to plural form:

Copy the file paths
Copy the directory paths
Copy the filenames

The original singular form is maintained if only one file is selected (or hovered).

Implementation Details

  1. Enabled the desc text field in keymap.toml to support formats like wol{ves|f} or name{s}, accommodating both regular and irregular plural forms.

  2. The new system works as follows:
    a. Matches content inside {} and splits it.
    b. For plural form: Uses the first element of the split to replace the whole {}.
    Example: wol{ves|f} => wolves, name{s} => names
    c. For singular form:

    • Uses the second element of the split if available.
    • If no second element (no |), removes the entire {}.
      Example: name{s} => name, wol{ves|f} => wolf
  3. Updated the logic to call selected_or_hovered in advance when the c key is pressed, ensuring accurate capture of selected file count in the Which window.

  4. For the Help window, the default plural form is used to display help information.

Additional Notes

I'd be much appericiated if you can provide me any feedback. To be honest, this PR is slightly more complicated than I initially though, But it helps me know lots more cool features of this project.

@sxyazi
Copy link
Owner

sxyazi commented Sep 26, 2024

Sorry, but I'm afraid I can't accept this PR, it's too complex for me for this single feature, and every time you get the key description, it requires running a regex match, which is time-consuming. Plus, when there's a match, it needs to replace them, requiring new memory allocation.

Passing selected_or_hovered to the which component and the config parsing also somewhat breaks the DDD domain boundaries, which is something I wanted to avoid in the design.

If we really want to fix this issue, the simplest way is to change the description to "Copy the file path(s)" as users usually know what they're doing - whether they've selected multiple files, so that should be enough.

@yanCode
Copy link
Contributor Author

yanCode commented Sep 26, 2024

no problem, let me close this PR.

@yanCode yanCode closed this Sep 26, 2024
@yanCode yanCode deleted the fix/plural branch September 26, 2024 12:38
@yanCode yanCode restored the fix/plural branch September 26, 2024 12:38
@yanCode yanCode deleted the fix/plural branch September 26, 2024 12:38
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