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

Make control-c to preserve the contents of the line and start new line #2904

Closed
obfuscated opened this issue Apr 5, 2016 · 39 comments
Closed
Assignees
Milestone

Comments

@obfuscated
Copy link

In bash if you type something and press ctrl-c then the content of the line is preserved and the cursor is moved to a new line. In fish the ctrl-c just clears the line. For me the behaviour of bash is a bit better, because it allows me to type something then press ctrl-c and I have the typed string in the log for further reference.

It looks something like:

$ cd
$ ls -l ^C
$ ls -la

Would it be possible to add this feature in fish, I'm fine if this is a non default configuration option.

@ridiculousfish
Copy link
Member

Makes sense to me

@krader1961
Copy link
Contributor

It's a key binding:

bind \cc 'commandline ""'

The obvious solution is to change the binding to insert a "#" at the start of the line to turn it into a comment then enter the command. Unfortunately, I can't find a way to make the following work:

bind \cC "commandline -f beginning-of-line; commandline -i '#'; commandline -f end-of-line; commandline -f \r"

The insertion occurs before moving to the beginning of the line. It looks like some enhancements will be needed to make this work. I'm going to leave this labeled as a question for a couple of days to see if someone like @faho knows how to make this work using the current fish code.

@floam
Copy link
Member

floam commented Apr 5, 2016

I don't know why we want to prepend a comment.

This works for me: bind \cc 'echo; commandline "";'

I feel this is less jarring behavior than the default.

This puts it in history but is noisy:
bind \cc 'echo; commandline | cat; commandline ""; commandline -f repaint'

I stole that from the mailing list I think.

@faho
Copy link
Member

faho commented Apr 6, 2016

The obvious solution is to change the binding to insert a "#" at the start of the line to turn it into a comment

He... I've been playing around with this just yesterday, and what I came up with is

function switch-comment-commandline --description 'Comment/Uncomment the current or every line'
    set -l cmdlines (commandline)
    set -l cmdlines2 (commandline -c)
    if test "$cmdlines" = "$cmdlines2"
        commandline -r (printf '%s\n' '#'$cmdlines | string replace -r '^##' '')
    else
        set -l linenum (count $cmdlines2)
        set cmdlines[$linenum] (string replace -r '^##' '' -- '#'$cmdlines[$linenum])
        commandline -r $cmdlines
    end
end

The heart of the matter being commandline -r (printf '%s\n' '#'$cmdlines | string replace -r '^##' ''). The rest is pretty much all to support multi-line commandlines.

For this case, all you really need is something like commandline -r '#'(commandline) and an additional commandline -f execute for the binding.

@krader1961
Copy link
Contributor

Thanks, @faho and @floam. I'll play with those solutions today and let you know if I find any problems. This capability is something I got into the habit of using long ago when I switched to ksh and then zsh and missed having in fish.

I'm also going to give some thought to how we can make these types of user contributions easier to discover. In the recent past I've seen several examples of this sort that don't warrant becoming default behavior but should be documented (outside of github issues) so people can more readily find them. Perhaps we need something like a "share/user_contrib" directory and someplace in the documentation to mention them.

@krader1961
Copy link
Contributor

The only solution that works (mostly) as advertised for me is bind \cc 'echo; commandline ""'. I say "mostly" because if you enter

if true
    echo yes

then press [ctrl-C] you're left with just the first line visible. @floams's second binding doesn't put anything into my command history. That's the point of the "turn the partial input into a comment" approach. If you do that then the comment gets inserted into the command history which makes it retrievable. But I couldn't get @faho's function to work in vi-mode.

@ghost
Copy link

ghost commented Apr 7, 2016

An improved echo:

EDIT:

bind \cc 'commandline -C 2147483647; for i in (seq (commandline -L)); echo; end; commandline ""'

@faho
Copy link
Member

faho commented Apr 7, 2016

But I couldn't get @faho's function to work in vi-mode.

Yeah, I've got some weird issues there as well. Will investigate. Though one binding that works for me is bind s 'switch-comment-commandline' -m default. If I change that to "\cs" it doesn't.

@faho
Copy link
Member

faho commented Apr 7, 2016

@krader1961: I've managed to get it to work - I always confuse "-m" and "-M".

Can you try bind -M default \cs 'switch-comment-commandline'?

@krader1961
Copy link
Contributor

@faho: The problem is that it's always taking the "else" branch and that code only affects the line with the cursor. So it works for a single line but not multiple lines. Not sure what your intent was but when I used this feature in ksh93 it always comments the entire command. So I just simplified your function to the following and get the behavior I expect:

set -l cmdlines (commandline)
commandline -r (printf '%s\n' '#'$cmdlines | string replace -r '^##' '')
commandline -f execute

@krader1961
Copy link
Contributor

@jakwings: That's the ticket. The only thing I changed was adding "^C" to make the output resemble what you get from bash which makes it clearer the command was canceled:

function cancel-commandline
    commandline -C 2147483647
    for i in (seq (commandline -L))
        echo '^C'
    end
    commandline ""
end

That definitely seems like something we should make the fish default. Whereas @faho's function is probably better left as a "user contributed, unsupported" function.

@krader1961 krader1961 added this to the next-2.x milestone Apr 7, 2016
@krader1961 krader1961 self-assigned this Apr 7, 2016
@floam
Copy link
Member

floam commented Apr 7, 2016

Wow, I love that!

I am going with this, which displays ^C how I like and shows it only once if I hit it while editing a multiline prompt (like editing this function), instead of on the line I was editing and the beginning of every line thereafter. Finally, we don't want to spam the terminal when used on an empty commanline.

function cancel-commandline
    if not test (commandline) = "" 2> /dev/null
        commandline -C 2147483647
        for i in (seq (commandline -L))
            if test $i -eq 1
                tput rev
                echo "^C"
                tput sgr0
            else
                echo ""
            end
        end
       commandline ""
       echo ""
    end
end

edit: fix test, copy @krader1961's extra echo.

@Darkshadow2
Copy link
Contributor

@krader1961 That almost works. If you have a single line command that is long enough to wrap, it only echoes the first "line" and the rest is lost (and the ^C is not shown).

@krader1961
Copy link
Contributor

Hmmm, yes, but the line doesn't have to literally wrap. It's enough for it to be longer than the space remaining after the prompt thus forcing fish to move the command to a line below the prompt. The best I can do is add an extra echo:

function cancel-commandline
    commandline -C 2147483647
    for i in (seq (commandline -L))
        echo '^C'
    end
    echo ''
    commandline ""
end

There doesn't seem to be a way to tell if fish has repositioned the text onto the line following the prompt. The above function adds an extra blank line when cancelling a single non-wrapped line. It produces the desired output for a wrapped (repositioned) line as well as if there are multiple lines.

@Darkshadow2
Copy link
Contributor

Hmm, maybe this:

function cancel-commandline
   set -l len (string length -- (commandline))
   if test $len -eq 0; return; end
   commandline -C 2147483647
   for i in (seq (commandline -L))
      echo '^C'
   end
   while test (math $len - $COLUMNS) -gt 0
      echo
      set len (math $len - $COLUMNS)
   end
   commandline ""
end

That keeps it from having an empty line if short, but isn't perfect since it doesn't take into account the length of the prompt.

@faho
Copy link
Member

faho commented Apr 8, 2016

The problem is that it's always taking the "else" branch and that code only affects the line with the cursor. So it works for a single line but not multiple lines. Not sure what your intent was but when I used this feature in ksh93 it always comments the entire command.

I don't see that. The idea was that you'd want to comment or uncomment the entire buffer if the cursor is at the end, or just the line you are at if it is not.

commandline and commandline -c should be equal if there's nothing after the cursor to cut, which should mean that the cursor is at the very end. And that's just how it works for me.

floam pushed a commit to floam/fish-shell that referenced this issue Apr 16, 2016
@ridiculousfish
Copy link
Member

This is a nice fix! But the bright yellow is quite striking. I would like to make it gray instead. Any agreement or objections?

@krader1961
Copy link
Contributor

The yellow was intentional since in western cultures it is associated with concepts like "warning" and "caution" and the strong visibility of the bright yellow makes it really obvious the pending command was cancelled. On the other hand until we can make set_color behave more sensibly with respect to eight color terminals grey is a safer choice.

@floam
Copy link
Member

floam commented Apr 29, 2016

I like the yellow, it never struck me as icky. I'd also not mind red - I want it visually stand out as an alarm-ey looking thing. Maybe it's just my colors?

screenshot 2016-04-29 at 2 02 36 pm

I don't mind grey - my version of this often looked grey. I had done this: tput rev; echo "^C"; tput sgr0

screenshot 2016-04-29 at 2 04 33 pm

I prefer actually inverting so that a user's terminal background color is unlikely to conflict with it. With this solution you are hard-pressed to accidentally have it not appear highlighted. And it generally looks like you expect, this is what zsh does.

screenshot 2016-04-29 at 2 04 24 pm

screenshot 2016-04-29 at 2 05 53 pm

screenshot 2016-04-29 at 2 07 32 pm

Is there a good reason why I shouldn't make set_color capable of this?

@gechr
Copy link
Contributor

gechr commented Apr 29, 2016

Why not just make it configurable, like other colours? Then the default doesn't really matter, as long as it's sensible.

Different people have different terminals, with varying capabilities & colour schemes. You can't cater to everyone out of the box.

@krader1961
Copy link
Contributor

As @floam points out we really shouldn't be hardcoding the color in that script. We should be using one of the colors from the theme selected by the user. I didn't do that in my original commit because none of the existing theme color variables made sense semantically.

@floam
Copy link
Member

floam commented Apr 29, 2016

Unfortunately a theme can't know what color someone's terminal uses as a background color. That's why I prefer tput rev -- it'd be nice if fish could do that where it works else fallback, through some kind of set_color inverted or something.

You can't cater to everyone out of the box.

Fish can and should cater where we can figure out how, out of the box, to a large proportion of users, and at least not suck for the remainder.

@krader1961
Copy link
Contributor

Unfortunately a theme can't know what color someone's terminal uses as a background color.

True, but presumably the user is using a theme that takes their terminal background color into account. But that returns us to my point that all of the existing theme color vars appear to be intended solely for fg use. In fact, I can't find any use of set_color -b in the standard fish scripts other than the one I added to _share/functions/_fish_cancel_commandline.fish. Which surprises me.

So I'm inclined to agree with @floam that the only truly portable way to handle this that has the desired visual impact is to add something like a --reverse option to set_color or a pseudo-color named reverse and use that.

@krader1961
Copy link
Contributor

I'll add a change to wrap the "^C" in (tput smso) and (tput rmso) and open an issue to track enhancing set_color. The cost of invoking an external program shouldn't be an issue in this case since it will be done infrequently and tput is pretty fast.

@ridiculousfish
Copy link
Member

ridiculousfish commented Apr 29, 2016

We don't need a big warning that the command was cancelled (zsh prints nothing at all here). It's fine to report it, but I find that the bright yellow draws too much of my attention and is distracting.

I like foreground gray (aka "brgrey"), which will match the newline indicator:

screen shot 2016-04-29 at 2 59 33 pm
for reference, current behavior
screen shot 2016-04-29 at 3 08 46 pm
I also like the idea of using Unicode here, perhaps ⦰. Though this will require some machinery to check whether Unicode is supported.
screen shot 2016-04-29 at 3 10 44 pm
As krader suggests, let's also make this a configurable color, which can cover both the newline and cancel signs. Maybe "fish_line_sigils"?

@krader1961
Copy link
Contributor

Let's open a new issue to discuss enhancing themes to include support for background colors and special characters for special situations. Obviously we'll want a cheap and easy way for for functions to determine if they can safely use non-ascii chars. In the meantime, so we can get 2.3.0 out the door with this facility, the safest solution is the one @floam proposed that I mentioned in my previous comment: use tput smso to just invert fg/bg.

@floam
Copy link
Member

floam commented Apr 29, 2016

I'm sorry, by zsh's behavior, I meant what it does with special characters in the line editor.
screenshot 2016-04-29 at 3 20 18 pm

@floam
Copy link
Member

floam commented Apr 29, 2016

I do like the unicode idea. But I don't think it should be dim.

@ridiculousfish
Copy link
Member

FYI this new control-C behavior is not in 2.3.0

@floam
Copy link
Member

floam commented Apr 29, 2016

The dimmed styling like we have when used for the newline indicator thing makes sense to me - but I think something marking an action, input by a user, that caused something, should not have such a passive styling - it could probably be less intense than what reverse does (I personally want it intense and a special alarm color, for some reason) - I really don't like it looking the exactly the same as the newline thing.

@krader1961
Copy link
Contributor

FYI this new control-C behavior is not in 2.3.0

You're right. I thought Zanchey included it when he cherry-picked the comment/uncomment change. Nonetheless, until we can reach a consensus on how to augment themes to support these types of use cases and so this works sensibly with eight color terminals like the Linux console VT we should just use tput to set standout mode for the ^C indicator.

@ridiculousfish
Copy link
Member

Sure, let's try the standout mode and see how it feels.

@floam
Copy link
Member

floam commented Apr 29, 2016

(FWIW I want to avoid confusing arguments for my color preferences versus what I think is the better principled shipping behavior - so I'll just say I do still think I personally prefer a yellow or red background (with white text, even) over what I get from tput rev/tput smso - that is the best thing to my taste - but it is better for me to overwrite in my configuration later or for a theme to do when we let them. The color-agnostic standout/reverse mode (what's the difference, practically, anyhow?) is the better default though. (Also, I've said it before but I'll say it again - I think it would be nice if for that "dimmed" newline indicator thing, and autosuggestions, if fish effectively did tput dim instead of assuming a color unless a theme thinks it knows better. This is another thing set_color can't do.)

@krader1961
Copy link
Contributor

I agree with you @floam. The nice thing about fish is it's trivial to put a copy of a standard function in your personal ~/config/fish/functions directory and hack it to suit one's personal preferences so if I want bright yellow I can still have that while everyone else gets a reasonable experience :-)

krader1961 added a commit that referenced this issue Apr 30, 2016
There was an extended discussion in #2904 about using a bright yellow background to make the cancelled command indicator, ^C, standout. The upshot was that standout (i.e., reversing fg/bg colors) mode should be used until themes are agumented with proper support for background colors and special characters.
krader1961 added a commit that referenced this issue Apr 30, 2016
There was an extended discussion in #2904 about using a bright yellow background to make the cancelled command indicator, ^C, standout. The upshot was that standout (i.e., reversing fg/bg colors) mode should be used until themes are agumented with proper support for background colors and special characters.

(cherry picked from commit a897ef0)
@gechr
Copy link
Contributor

gechr commented May 1, 2016

[...] a copy of a standard function in your personal ~/config/fish/functions directory [...]

@krader1961 While this works, I've personally been stung by doing it in the past. The problem is, you then stop receiving enhancements/fixes for that function and (in my case) completely forget you've taken a copy of the buggy default in the first place, which subsequently gets fixed upstream, and you don't receive the update.

In this case, would it not be better to add an empty variable in place of the colour (e.g. $__fish_cancel_commandline_colour), which can then be set in the user's config.fish? Then, all that needs to be set is the variable, rather than a copy of the whole function. This is the pattern __fish_git_prompt follows and it works well.

@faho faho modified the milestones: 2.3.0, next-2.x May 1, 2016
@floam
Copy link
Member

floam commented Jul 16, 2016

I'll add a change to wrap the "^C" in (tput smso) and (tput rmso) and open an issue to track enhancing set_color.

@krader1961 did you file that?

@cjxgm
Copy link

cjxgm commented Nov 20, 2016

I hate the new behavior. How can I customize it back to the old "wipe out the current line" behavior?

@krader1961
Copy link
Contributor

@cjxgm: Just add the following to your personal fish config:

function fish_user_key_bindings
    bind \cc 'commandline ""'
end

In hindsight this should have been added to the fish FAQ.

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Apr 3, 2017

I added

function fish_user_key_bindings
    bind \cc 'commandline ""'
end

but there's must be something wrong: it behaves very strangely. Probably because I have a two-lines prompt. Sometimes ^C clears the line (old behavior), sometimes it starts a new line (new behavior) and sometimes it overwrites the last line with the current and start a new line.

ridiculousfish added a commit to ridiculousfish/fish-shell that referenced this issue Feb 7, 2018
In commit 5f849d0, issue fish-shell#2904, the control-C behavior was changed to
print an inverted ^C and then a newline. This behavior has not been
well-received, judging by positive reactions for how to undo it. This
commit resets the behavior to the original behavior of clearing the command
line.
ridiculousfish added a commit that referenced this issue Feb 7, 2018
In commit 5f849d0, issue #2904, the control-C behavior was changed to
print an inverted ^C and then a newline. This behavior has not been
well-received, judging by positive reactions for how to undo it. This
commit resets the behavior to the original behavior of clearing the command
line.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants