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

Add 'sd' replace method #214

Merged
merged 1 commit into from
Mar 22, 2024
Merged

Add 'sd' replace method #214

merged 1 commit into from
Mar 22, 2024

Conversation

Zananok
Copy link
Contributor

@Zananok Zananok commented Mar 21, 2024

No description provided.

@EpsilonKu EpsilonKu merged commit 31f62d7 into nvim-pack:master Mar 22, 2024
2 of 3 checks passed
@mathjiajia
Copy link

I guess you need to modify the following:

if vim.loop.os_uname().sysname == 'Darwin' then
config.replace_engine.sed.cmd = 'gsed'
if vim.fn.executable('gsed') == 0 then
print("You need to install gnu sed 'brew install gnu-sed'")
end
end

@Zananok
Copy link
Contributor Author

Zananok commented Mar 22, 2024

This will change the defaults. My PR is to add 'sd' support, not override the default. Additionally, your line is about changing the replace_engine of sed. The replace engine of sd is always sd, as its installed by the user which means it will always be the same no matter the uname.

To enable sd (in your personal lua config):

require("spectre").setup {
    default = {
        replace = {
            cmd = "sd"
        }
    }
}

Let me know if I misunderstood your comment and that indeed I should add/change anything.

@mathjiajia
Copy link

mathjiajia commented Mar 22, 2024

No. Even if I set sd as default,
these piece of code still will run on macOS

if vim.loop.os_uname().sysname == 'Darwin' then
config.replace_engine.sed.cmd = 'gsed'
if vim.fn.executable('gsed') == 0 then
print("You need to install gnu sed 'brew install gnu-sed'")
end
end

In the case I removed gnu-sed, it will print that notification.

It should execute only in the case that user sets default replacement tool as sed.

@Zananok
Copy link
Contributor Author

Zananok commented Mar 22, 2024

Aha, I understand what you mean now.

However, this code would also run if one set the replace cmd to 'oxi', which is another alternative. I would report it as a bug and not that the current PR is missing.

Though, since I am already in the headspace, I can make another PR fixing the issue 👍.

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.

3 participants