jorgebucaran / fisher

A plugin manager for Fish
https://git.io/fisher
MIT License
7.55k stars 257 forks source link

Enhancement: Write fish_plugins in alphabetical order #689

Closed thernstig closed 2 years ago

thernstig commented 2 years ago

It would be nice if fish_plugins could be written in alphabetical order.

It is a small thing and the reason is that anyone using dotfile managers to persist state (the config) for fish plugins, the fish_plugins shows as changed in diffs since it jumbles the file every time a fisher update X is run for a plugin. So it is a quality of life improvement for some of us.

jorgebucaran commented 2 years ago

I'm not sure that's something easy to implement in just Fish, but maybe you've thought of something?

It would be nice to be able to keep Fisher 100% pure Fish.

thernstig commented 2 years ago

Fish shows usage of sort in their own docs, but I suppose that is not 100% pure fish and thus something you want to avoid? https://fishshell.com/docs/current/cmds/string.html#nul-delimited-examples

I do see you use e.g. mkdir so maybe sort is fine then?

jorgebucaran commented 2 years ago

Let's collect more feedback here before introducing this change. 🙏

evanrelf commented 2 years ago

it jumbles the file every time a fisher update X is run for a plugin

I experienced the same issue, which led me to this issue.

I think it's safe to assume nearly every machine has sort installed. And if they don't, then you can just skip sorting of the file.

ismay commented 2 years ago

I agree, sort seems fine to me as well. Plus, as @evanrelf said you can just skip it if sort isn’t on the host for some reason.

jorgebucaran commented 2 years ago

the fish_plugins shows as changed in diffs since it jumbles the file every time a fisher update X is run for a plugin

Does anyone how to reproduce this? Can't seem to get them jumbled up no matter how many times I run fisher update. I'd like to know, because maybe another way to solve this issue would be by preserving plugin order between runs, rather than writing them out in alphabetical order.

ismay commented 2 years ago

For me just running a regular update did it. But it doesn't happen every time. Not sure yet what's responsible for the entries getting reordered.

edit: it seems like this is where the changes to the plugins are being persisted to the plugin file right: https://github.com/jorgebucaran/fisher/blob/main/functions/fisher.fish#L187? I've not gone through it in depth, but I can imagine that this would cause plugins to be reordered. It doesn't seem like there's any logic there to prevent the existing order from being changed. Or am I reading it wrong?

ismay commented 2 years ago

One thing you could do if you want to avoid sort but still resolve the issue, is to not change the plugins in the fish_plugins file for an update. So:

That way, it resolves the original issue: i.e., not messing up the order for an update. And then if the user wants to sort the plugins they are free to do so themselves and fisher will leave it intact. Basically, fisher shouldn't touch the plugins file for an update. Would that work?

ismay commented 2 years ago

@jorgebucaran Having said that, in skimming through the code it seems like the logic for installation is somewhat mixed with that for updating. I can imagine that the simplest solution would be to just check if sort is available here: https://github.com/jorgebucaran/fisher/blob/main/functions/fisher.fish#L187, and use that if it exists.

That way it will degrade gracefully if sort can't be found. And if it can be found, the plugins will be sorted for both installations and updates, which seems nice. That seems like a practical way forward.

jorgebucaran commented 2 years ago

Without a native native sort solution in Fish, I see this feature unlikely to land here anytime soon, but I'm still open to ideas.

thernstig commented 2 years ago

@jorgebucaran let's see what they say: https://github.com/fish-shell/fish-shell/issues/9013

What is the reluctance to check if sort exists as per @ismay's suggestion? Or do what is suggested at https://github.com/jorgebucaran/fisher/issues/689#issuecomment-1080293665 ?

thernstig commented 2 years ago

@jorgebucaran apparently it already exists soonish, but it'd require users to have a specific version of fish.

See https://github.com/fish-shell/fish-shell/pull/8958

You can use path sort to sort paths but also arbitrary strings.

Seems to land in fish 3.5 which seems very close to being finalized: https://github.com/fish-shell/fish-shell/milestone/32

kidonng commented 2 years ago

My take on pure™️ fish sort that may fulfill fisher's need[^1]:

function naive_sort
    test (count $argv) != 0 || return
    set -l dir (mktemp -d)

    mkdir -p $dir/$argv
    set -l glob (string replace -a $dir/ "" $dir/**)

    set -l sorted
    for i in $argv
        test -n "$i" && set sorted[(contains -i $i $glob)] $i
    end

    string match -v "" $sorted
    rm -r $dir
end

function naive_sort_slashed
    set -l slashed (string replace -rf "^/(.)" '$1' $argv)
    test (count $slashed) != 0 && printf /%s\n (dump_sort $slashed)
    dump_sort (string match -v "/*" $argv)
end

$ naive_sort_slashed jorgebucaran/fisher ilancosman/tide@v5 jorgebucaran/nvm.fish /home/jb/path/to/plugin
/home/jb/path/to/plugin
ilancosman/tide@v5
jorgebucaran/fisher
jorgebucaran/nvm.fish

Alright, just joking. However this can be made shorter and has better compatibility with older fish than the upcoming path sort, so...

[^1]: fisher already uses mktemp, mkdir and rm so no extra dependency

jorgebucaran commented 2 years ago

@ismay An update will not change the list of plugins (since the plugins should already be present)

I don't think that would work. If you add a new plugin (one currently not installed) to your fish_plugins and run fisher update, that will be the same as installing said plugin and updating all the already installed plugins.

jorgebucaran commented 2 years ago

I feel like the correct solution to the problem is not writing out plugins in alphabetical order, but preserving plugin order.

jorgebucaran commented 2 years ago

Meanwhile, here's a (rather slow) quicksort in Fish—just for fun.

function ord --description "Convert a string to decimal values"
    printf %d\n '"'$argv
end

function fish_sort --argument-names head --description "Non-in-place, recursive pure Fish slow quicksort"
    count $argv >/dev/null || return
    set --local lesser
    set --local greater

    for elem in $argv[2..-1]
        set --local elem_dec (ord (string split -- "" $elem))
        set --local head_dec (ord (string split -- "" $head))
        set --local len (math "min("(count $elem_dec)","(count $head_dec)")")

        for j in (seq $len)
            test $elem_dec[$j] -eq $head_dec[$j] &&
                test $len -eq $j && set --append lesser $elem && continue
            test $elem_dec[$j] -lt $head_dec[$j] &&
                set --append lesser $elem || set --append greater $elem
            break
        end
    end

    printf "%s\n" (qsort $lesser) $head (qsort $greater)
end
thernstig commented 2 years ago

@jorgebucaran preserving order is what main problem has been (at least for me) all time along, and probably others reading this thread. So I think that is a good approach to take if you can fix that.

Though in general I like sorting of strings as it makes it quick to find things in large lists, but that is not as important.

jorgebucaran commented 2 years ago

it jumbles the file every time a fisher update X is run for a plugin

I am afraid I still can't reproduce this. Any tips? What OS are you using?

Are you sure the diff you saw was not a result of interactively removing and re-installing a plugin, therefore changing its position in fish_plugins?

cc @evanrelf @ismay @thernstig

thernstig commented 2 years ago

@jorgebucaran

> fisher update jethrokuan/fzf
> chezmoi diff
diff --git a/.config/fish/fish_plugins b/.config/fish/fish_plugins
index d0249f506bff85af351d66e91e14fc234f8f777b..3cab39bfee10f237cd2c536a92b571973a4a9b8e 100664
--- a/.config/fish/fish_plugins
+++ b/.config/fish/fish_plugins
@@ -1,4 +1,4 @@
+jethrokuan/fzf
 jorgebucaran/fisher
 thernstig/fish-ssh-agent
 jorgebucaran/nvm.fish
-jethrokuan/fzf

See how jethrokuan/fzf got placed at the end.

thernstig commented 2 years ago

Nice @jorgebucaran :)