Quote minimap_gen command path

garyttierney commented 3 years ago

It was previously failing to execute if the path contained invalid characters (e.g. spaces).

Check list

[x] I have read through the README (especially F.A.Q section)
[x] I have searched through the existing issues or pull requests
[ ] I have verified all existing unit tests pass (See the README on how to run)
[ ] I have added new tests if appropriate
[ ] I have performed a self-review of my code and commented hard-to-understand areas
[ ] I have made corresponding changes to the documentation (when necessary)

Description

The plugin was previously failing to execute the minimap generation command if the path to the binary contained characters that the shell would interpret as a word splitter. This change quotes commands before running them.

Type of change

[x] Bug fix
[ ] New feature
[ ] Improvement of existing features
[ ] Refactor
[ ] Breaking change
[ ] Documentation change
[ ] CI / CD

Test environment

OS
- [ ] Linux
- [ ] Mac OS X
- [x] Windows
- [ ] Others:
Vim
- [x] Neovim: 0.5.0
- [ ] Vim:

There are some failing tests, but don't think they're related to this change. They appear to fail with the same reason even without a working minimap:

✗ Search - Beginning of file
    Expected "[]" to equal "[[1, 1, 3], [3, 4, 3], [3, 10, 3], [3, 13, 3]]"
        <SNR>72_minimap_test_search_beginning_of_file:5
✗ Search - Beginning of line
    Expected "[]" to equal "[[1, 4, 3], [3, 1, 3], [3, 1, 3]]"
        <SNR>72_minimap_test_search_beginning_of_line:7
✗ Search - End of line
    Expected "[]" to equal "[[1, 4, 3], [3, 7, 3]]"
        <SNR>72_minimap_test_search_end_of_line:5
✗ Search - Multi match per line
    Expected "[]" to equal "[[3, 4, 3], [3, 16, 3], [3, 22, 3]]"
        <SNR>72_minimap_test_search_multi_per_line:5
√ Search - Spanning a line
✗ Search - Many results
    Expected "[]" to equal "[[1, 1, 3], [1, 4, 3], [1, 19, 3], [1, 22, 3], [1, 25, 3], [1, 25, 3], [1, 1, 3], [1, 1, 3], [1, 1, 3], [2, 1, 3], [2, 1, 3], [2, 1, 3], [2, 1, 3], [3, 1, 3], [3, 1, 3], [3, 1, 3], [3, 1, 3], [3, 1, 3], [3, 16, 3]]"
        <SNR>72_minimap_test_search_many_results:8
✗ Search - Long Match
    Expected "[]" to equal "[[1, 7, 21]]"
        <SNR>72_minimap_test_search_long_match:5
✗ Search - History
    Expected "[]" to equal "[[1, 1, 3], [1, 1, 3], [1, 1, 3], [2, 1, 3], [2, 1, 3], [2, 1, 3], [2, 1, 3], [3, 1, 3], [3, 1, 3]]"
        <SNR>72_minimap_test_search_history:11

ZNielsen commented 3 years ago

I am not able to run the unit tests on windows, do they pass on the master branch?

garyttierney commented 3 years ago

Ah, the issue was a Unix path in the test and I'm on Windows. I updated it to write to a tempname() file but see these failures now:

✗ Search - Beginning of file
    Expected "[[1, 1, 3], [1, 64, 3], [1, 79, 3], [1, 82, 3]]" to equal "[[1, 1, 3], [3, 4, 3], [3, 10, 3], [3, 13, 3]]"
        <SNR>72_minimap_test_search_beginning_of_file:5
✗ Search - Beginning of line
    Expected "[[1, 7, 3], [1, 61, 3], [1, 70, 3]]" to equal "[[1, 4, 3], [3, 1, 3], [3, 1, 3]]"
        <SNR>72_minimap_test_search_beginning_of_line:7
✗ Search - End of line
    Expected "[[1, 4, 3], [1, 79, 3]]" to equal "[[1, 4, 3], [3, 7, 3]]"
        <SNR>72_minimap_test_search_end_of_line:5
✗ Search - Multi match per line
    Expected "[[1, 76, 3], [1, 88, 3], [1, 91, 3]]" to equal "[[3, 4, 3], [3, 16, 3], [3, 22, 3]]"
        <SNR>72_minimap_test_search_multi_per_line:5
√ Search - Spanning a line
✗ Search - Many results
    Expected "[[1, 1, 3], [1, 7, 3], [1, 19, 3], [1, 22, 3], [1, 25, 3], [1, 25, 3], [1, 28, 3], [1, 31, 3], [1, 37, 3], [1, 40, 3], [1, 43, 3], [1, 46, 3], [1, 49, 3], [1, 55, 3], [1, 58, 3], [1, 61, 3], [1, 61, 3], [1, 70, 3], [1, 85, 3]]" to equal "[[1, 1, 3], [1, 4, 3], [1, 19, 3], [1, 22, 3], [1, 25, 3], [1, 25, 3], [1, 1, 3], [1, 1, 3], [1, 1, 3], [2, 1, 3], [2, 1, 3], [2, 1, 3], [2, 1, 3], [3, 1, 3], [3, 1, 3], [3, 1, 3], [3, 1, 3], [3, 1, 3], [3, 16, 3]]"
        <SNR>72_minimap_test_search_many_results:8
√ Search - Long Match
✗ Search - History
    Expected "[[1, 28, 3], [1, 34, 3], [1, 37, 3], [1, 40, 3], [1, 43, 3], [1, 46, 3], [1, 52, 3], [1, 55, 3], [1, 58, 3]]" to equal "[[1, 1, 3], [1, 1, 3], [1, 1, 3], [2, 1, 3], [2, 1, 3], [2, 1, 3], [2, 1, 3], [3, 1, 3], [3, 1, 3]]"
        <SNR>72_minimap_test_search_history:11
√ Search tear down
√ Minimap conversion math works as expected
√ Opening minimap restults in only one call to update_minimap

garyttierney commented 3 years ago

I am not able to run the unit tests on windows, do they pass on the master branch?

Nope, the failures above are from master (after fixing the path issue).

ZNielsen commented 3 years ago

Expected "[[1, 7, 3], [1, 61, 3], [1, 70, 3]]" to equal "[[1, 4, 3], [3, 1, 3], [3, 1, 3]]" Expected "[[1, 76, 3], [1, 88, 3], [1, 91, 3]]" to equal "[[3, 4, 3], [3, 16, 3], [3, 22, 3]]" It looks like all your matches are happening on the first line.

let tmp = split(execute('his /'), '\n')

We are splitting on \n, looks like we need to add platform dependent code to also split on \r\n. I'll open a separate issue for that.

wfxr / minimap.vim