Custom Super User Command

[savaroskij]

This PR adds and documents the possibility to set a custom super user command to prepend to the command line, such as doas instead of sudo for *BSD users.

You can set the custom su command in the variable su_command.

[bobthecow]

This seems entirely reasonable. I would probably use sudope_command as the variable name, to keep it namespaced by the plugin that consumes it.

[savaroskij]

Thanks @bobthecow, that's a good idea. Done this in 47e6946.

[bobthecow]

maybe set -l for the fallback here? does anything outside this function need it?

[savaroskij]

Sorry, I'm new to fish, but I can read in help set:

If a variable is not explicitly set to be either universal, global or
local and has never before been defined, the variable will be local to
the currently executing function. Note that this is different from
using the -l or --local flag. If one of those flags is used, the
variable will be local to the most inner currently executing block,
while without these the variable will be local to the function. If no
function is executing, the variable will be global.

So I think that all these -l flag in this file are not useful. Correct me if I'm wrong, please.

[bobthecow]

using unspecified variable scope introduces opportunities to pollute global state, e.g.:

set -gx sudope_command

echo $sudope_command # nothing

function thing
  if test -z "$sudope_command"
    set sudope_command sudo
  end
end

thing

echo $sudope_command # "sudo"

if you used set -q rather than test -z to check for a user-defined value, it would have only set the fallback locally. but it also would have failed if $sudope_command were set to empty, as above.

i tend to use a pattern that always sets locally, to avoid any chance of this sort of thing. in this case, we could either introduce a brand new variable name, or we can use set -l outside a block:

set -gx sudope_command

echo $sudope_command # nothing

function thing
  test -z "$sudope_command"
    or set -l sudope_command sudo
end

thing

echo $sudope_command # nothing

[savaroskij]

Now I understand. Thanks for your explanation and your patience.

[savaroskij]

Fixed in 18d9fd2.

[bobthecow]

we should probably regex escape $sudope_command (string escape --style=regex -- "$sudope_command" and store it somewhere?)

[savaroskij]

I didn't know the string builtin. Cool stuff, thanks! Can you explain to me its role here? $sudope_command it's supposed to be a shell command (eg. sudo, doas, et cetera). Are you thinking about something like mkfs.ext2, that in fact it's a problem here?

[bobthecow]

yeah, . was the thing i was most interested in. but mostly it's defensive programming. interpolating user-provided values into a regex introduces opportunities for things to go wrong, and it's unnecessary when we have a way to escape them right here :)

[savaroskij]

Perfect, I'm with you :)

[savaroskij]

Fixed in 11b1fca.

[bobthecow]

set -l here?

[savaroskij]

Again from help set:

If a variable is not explicitly set to be either universal, global or
local, but has been previously defined, the previous variable scope is
used.

[bobthecow]

right. i thought we wanted this one to be block scoped? do we want it to persist after this case block?

[savaroskij]

In the contest of your reply above (#15 (comment)), it makes no sense to make it persistent. I'll fix all these issues in my next commits. Thanks.

[savaroskij]

Fixed in 18d9fd2.

[savaroskij]

e1492c6 fixes a bug introduced by previous changes in my branch: when the super user command was added to the command line, the following white space was not considered in pushing the cursor ahead.