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

added possibilty to paste a directory #41

Merged
merged 1 commit into from
Dec 28, 2020
Merged

Conversation

3ximus
Copy link
Contributor

@3ximus 3ximus commented Dec 28, 2020

Hi, as sugested in issue #40 it would be great to add a way to paste the path to a directory to make it easy to do these kinds of actions

$ ls `fzm`
$ cp file1.txt `fzm`

Then we can hit ctrl-k by default to paste the path and the command substitution will be replaced by the bookmark path when the comand gets executed.
Similar to what you did with the delete key, the default pasting key can be modified with $FZF_MARKS_PASTE

I don't think its possible through this script to be able to hit Ctrl-g and directly paste the path as sugested in the issue..

@urbainvaes
Copy link
Owner

Looks really good, thanks!

@urbainvaes urbainvaes merged commit 4000f29 into urbainvaes:master Dec 28, 2020
@urbainvaes
Copy link
Owner

urbainvaes commented Dec 28, 2020

For some reason, I think the example does not work in bash. Have you tested in both bash and zsh? Also, do you know whether it is necessary to define a widget with zle -N pmark? I tried your example without this line, and it seems to work fine.

urbainvaes added a commit that referenced this pull request Dec 28, 2020
@urbainvaes
Copy link
Owner

urbainvaes commented Dec 28, 2020

In the commit b0ff00d , I slightly modified your implementation of the feature in zsh. Now both

ls $(fsm)

and

ls <c-g><select-entry-with-fzf><c-k>

should behave as intended. I particular, it seems possible to paste the result after <c-g>. Let me know if you see issues with my implementation, and thanks again!

@3ximus
Copy link
Contributor Author

3ximus commented Dec 28, 2020

Hi, yeah, sorry I forgot to mention that I didn't try it on zsh since I don't use it, I just copied from bash and added zle -N pmark since it seemed necessary (didn't really check what it did).

But it does work on bash:
out

@3ximus
Copy link
Contributor Author

3ximus commented Dec 28, 2020

ls <c-g><select-entry-with-fzf><c-k>

should behave as intended. I particular, it seems possible to paste the result after <c-g>. Let me know if you see issues with my implementation, and thanks again!

That's really dope, I don't know if its possible to with bash readline,, but I'm using https://github.com/akinomyoga/ble.sh which improves bash's apabilities, I'll ask if there is a way to implement the same behavior since its quite useful.
Cheers

urbainvaes added a commit that referenced this pull request Dec 28, 2020
See pull request #41.
@urbainvaes
Copy link
Owner

urbainvaes commented Dec 28, 2020

But it does work on bash:

Ah sorry, I think wasn't testing properly (I think I was pressing Enter instead of c-k). I re-added the feature in bash.

I don't know if it's possible to with bash readline

I see that the default fzf bindings ctrl-t work like we want (see fzf-file-widget() here), so it should be possible. I don't have much time to look into this now, but feel free to implement it if you think it's worth it. :)

Many thanks again!

@3ximus
Copy link
Contributor Author

3ximus commented Dec 28, 2020

Yeah, I was just about to tell you that I realized they did what we wanted, however it conflicts with the default behavior when we want to jump into a directory (we cannot map the \n ) and it causes the prompt to stay the same until we manually reload it:
out

As you see once I pressed Enter it jumps to the directory but it doesn't cause the prompt to reload

In this case I'm binding like this:
bind -x "\"${FZF_MARKS_JUMP:-\C-g}\":\"fzm\""
Instead of before:
bind "\"${FZF_MARKS_JUMP:-\C-g}\":\"fzm\\n\""

And the pmark function like this:

function pmark {
  local selected="$(echo "${1}" | sed 's/.*: \(.*\)$/\1/' | sed "s#^~#${HOME}#")"
  READLINE_LINE="${READLINE_LINE:0:$READLINE_POINT}$selected${READLINE_LINE:$READLINE_POINT}"
  READLINE_POINT=$(( READLINE_POINT + ${#selected} ))
}

I'll make a pull request once I know of a way to avoid this behavior.
Do you have any idea how to fix it?

@urbainvaes
Copy link
Owner

urbainvaes commented Dec 28, 2020

Nice, this is amazing!

I agree with you that the only issue with this is that the prompt isn't redrawn when changing directory. I looked for solutions here, but the main ones don't seem to work...

I was hoping something like this would work,

bind '"\er": redraw-current-line'
bind -x '"\ex": fzm'
bind "\"${FZF_MARKS_JUMP:-\C-g}\":\"\\ex\\er\""

but it didn't...

@3ximus
Copy link
Contributor Author

3ximus commented Dec 29, 2020

Hi, I found a way around it !
We can bind like this:

bind -x '"\202": "fzm"'
bind '"\C-g": "\202 \C-a\C-k\C-m\C-y\C-h"'

This clears the internal bash clipboard (but i dont think its much of an issue), and always redraws in the next line:
out
From my research there doesn't seem to be any other way around it, and I'm ok with this solution...
I'll submit a pull request. Cheers !
👍

@urbainvaes
Copy link
Owner

Thank you very much for this, it looks great! :)

I don't think redrawing the next line and clearing the clipboards are big issues, but I noticed (unless I did something wrong in my test) that

ls $(fzm)

no longer works with this. Is this something you observed as well?

@3ximus
Copy link
Contributor Author

3ximus commented Dec 29, 2020

Yeah, its not going to work anymore since the pmark function sets the content of the of the prompt itself instead of printing the string. Now the way to use it is by hitting C-g, selecting the mark, hitting C-k and this will place the path at the cursor.

However there is an issue, if you paste the path in the middle of a command it will put the cursor at the end placing a space after the pasted text and deleting a character (i added this to the mapping avoid pasting the internal bash clipboard content). The gif demonstrates the issue:
out

There is a workaround that I wanted to avoid, which defines 2 macros to save the state of the inputed command (both before and after the cursor) , and if I bind them it works correctly:
out

However this means that I have to bind like this:

bind -x '"\200": TEMP_LINE=$READLINE_LINE; TEMP_POINT=$READLINE_POINT'
bind -x '"\201": READLINE_LINE=$TEMP_LINE; READLINE_POINT=$TEMP_POINT; unset TEMP_POINT; unset TEMP_LINE'
bind -x '"\202": "fzm"'
bind '"\C-g":"\202\200\C-a\C-k\C-m\201"'

It shouldn't cause any problems unless someone has these macros defined and it interferes with them. But we can assume that if they have, they'll know how to fix the problem, and it can be mentioned on the README file, I updated the pull request #43 with this fix.

About the use of ls $(fzm) I don't think its necessary to provide it anymore since this works much better imo, and I don't see a way of providing both functionalities...

@akinomyoga
Copy link
Collaborator

akinomyoga commented Dec 30, 2020

It shouldn't cause any problems unless someone has these macros defined

Unfortunately, the above bindings conflict with normal UTF-8 characters with the normal Bash (i.e., without ble.sh) because bytes such as \200, \201 and \202 are used for non-first bytes of multibyte characters in UTF-8. For example, when you input hiraganas (Japanese phonetic characters) with these bindings, the hiraganas will not be inserted in the command line but the commands bounded by \201 or \202 are invoked randomly because the UTF-8 representation of hiraganas is \343\201\??? and \343\202\???.

What is non-trivial is that bind builtin of the normal Bash interprets the specified key sequence (the left-hand side of : ) as a byte sequence rather than a character-code sequence, which may cause unintended conflicts and garbage characters in the command line. In ble.sh, the behavior has been changed so that the key sequence is interpreted as the character-code sequence, so the above bindings work as intended, but it's not the case of the normal Bash.

Actually, the available set of unused bytes depend on the character encoding of your terminal. If the terminal sends UTF-8 encoded characters, maybe you can think of using the UTF-8 representation of U+0080, U+0081, and U+0082, which are \302\200, \302\201, and \302\202, respectively. However, this may cause conflicts in other encodings. For example, in ISO 8859 encodings, \302 is usually some letter with an accent. [ Note: Even with UTF-8, there is still a very small possibility of conflicts because U+0080, etc. are actually a part of 8-bit representations of C1 control characters, but only a few terminals support 8-bit representation with UTF-8 encoding, and almost all the modern terminals send 7-bit representation of C1 characters by default. ]

@3ximus
Copy link
Contributor Author

3ximus commented Dec 30, 2020

Hum I see...
So there is no perfect way handle this I suppose, in that case do you suggest binding like this to cause the least amount of issues ?

bind -x '"U+0080": TEMP_LINE=$READLINE_LINE; TEMP_POINT=$READLINE_POINT'
bind -x '"U+0081": READLINE_LINE=$TEMP_LINE; READLINE_POINT=$TEMP_POINT; unset TEMP_POINT; unset TEMP_LINE'
bind -x '"U+0082": "fzm"'
bind '"\C-g":"U+0082U+0080\C-a\C-k\C-mU+0081"'

This seems to be working correctly... Any suggestions?

@urbainvaes
Copy link
Owner

Many thanks both for your input! I see that ctrl-t in fzf works well in bash even in the middle of a command. Is the workaround you are using the same as the one used in fzf, @3ximus?

@akinomyoga
Copy link
Collaborator

akinomyoga commented Dec 30, 2020

This seems to be working correctly... Any suggestions?

Maybe we can do something like this (not tested)?

function set-up-fzm-bindings {
  local mark1='\200' mark2='\201' mark3='\202'

  # Change markers for UTF-8 encodings
  local locale=${LC_ALL:-${LC_CTYPE:-$LANG}}
  local rex_utf8='\.([uU][tT][fF]-?8)$'
  if [[ $locale =~ $rex_utf8 ]]; then
    mark1='\302\200' mark2='\302\201' mark3='\302\202'
  fi

  bind -x "\"$mark1\": TEMP_LINE=\$READLINE_LINE TEMP_POINT=\$READLINE_POINT"
  bind -x "\"$mark2\": READLINE_LINE=\$TEMP_LINE READLINE_POINT=\$TEMP_POINT; unset -v TEMP_POINT TEMP_LINE"
  bind -x "\"$mark3\": fzm"
  bind "\"\C-g\":\"$mark3$mark1\C-a\C-k\C-m$mark2\""
}
set-up-fzm-bindings

BTW, what is the minimal Bash version that fzf-marks supports? I think the implementation should change depending on that. There are actually many version dependent things in this area: bind -x is only available from Bash 4.0, bind -x has a bug that it doesn't work with 3- or more-byte key sequences until Bash 4.2, etc.

@3ximus
Copy link
Contributor Author

3ximus commented Dec 30, 2020

Many thanks both for your input! I see that ctrl-t in fzf works well in bash even in the middle of a command. Is the workaround you are using the same as the one used in fzf, @3ximus?

Kind of, because ctrl-t is only used for pasting, and that part is handled similarly, but since we want ctrl-g to be able to both paste or change directory the bindings have to be a bit different in order to do both and be able to update the prompt.

function set-up-fzm-bindings {
  local mark1='\200' mark2='\201' mark3='\202'

  # Change markers for UTF-8 encodings
  local locale=${LC_ALL:-${LC_CTYPE:-$LANG}}
  local rex_utf8='\.([uU][tT][fF]-?8)$'
  if [[ $locale =~ $rex_utf8 ]]; then
    mark1='\302\200' mark2='\302\201' mark3='\302\202'
  fi

  bind -x "\"$mark1\": TEMP_LINE=\$READLINE_LINE TEMP_POINT=\$READLINE_POINT"
  bind -x "\"$mark2\": READLINE_LINE=\$TEMP_LINE READLINE_POINT=\$TEMP_POINT; unset -v TEMP_POINT TEMP_LINE"
  bind -x "\"$mark3\": fzm"
  bind "\"\C-g\":\"$mark3$mark1\C-a\C-k\C-m$mark2\""
}
set-up-fzm-bindings

This works 👍

The solutions that fzf has for ctrl-t and alt-c don't work perfectly for me at without the keybindings that use $READLINE_POINT and $READLINE_LINE to restore the prompt correctly, and it doesn't seem possible to have both features on the same keybinding without the macro saved with bind -x because we can either have bind

bind "\"\C-g\":\"fzm\C-m\"" # which wont be able to support placing text on the command line

or

bind "\"\C-g\":\"`fzm`\C-m\""
# which would be able to place the text but due to running on a subshell wouldn't be able to change directory


So I suggest that for Bash>4 we keep the macros with bind -x and for Bash<4 we have limited functionality (Use ctrl-g + Enter to jump and ctrl-q + ctrl-k simply echoes the path and can be used with a subcommand ls $(fzm) )

For this to work I modified the function pmark to have both cases and used @akinomyoga function for the bindings taking bash version into account:

function pmark {
  local selected="$(echo "${1}" | sed 's/.*: \(.*\)$/\1/' | sed "s#^~#${HOME}#")"
  if [ "${BASH_VERSINFO[0]}" -lt 4 ]; then
      printf '%q' "$selected"
  else
      READLINE_LINE="${READLINE_LINE:0:$READLINE_POINT}$selected${READLINE_LINE:$READLINE_POINT}"
      READLINE_POINT=$(( READLINE_POINT + ${#selected} ))
  fi
}
function set-up-fzm-bindings {
    if [ "${BASH_VERSINFO[0]}" -lt 4 ]; then
        bind "\"\C-g\":\"fzm\C-m\""
    else
        local mark1='\200' mark2='\201' mark3='\202'

        # Change markers for UTF-8 encodings
        local locale=${LC_ALL:-${LC_CTYPE:-$LANG}}
        local rex_utf8='\.([uU][tT][fF]-?8)$'
        if [[ $locale =~ $rex_utf8 ]]; then
          mark1='\302\200' mark2='\302\201' mark3='\302\202'
        fi
        bind -x "\"$mark1\": TEMP_LINE=\$READLINE_LINE TEMP_POINT=\$READLINE_POINT"
        bind -x "\"$mark2\": READLINE_LINE=\$TEMP_LINE READLINE_POINT=\$TEMP_POINT; unset -v TEMP_POINT TEMP_LINE"
        bind -x "\"$mark3\": fzm"
        bind "\"\C-g\":\"$mark3$mark1\C-a\C-k\C-m$mark2\""
    fi

    if [ "${FZF_MARKS_DMARK}" ]; then
        bind "\"${FZF_MARKS_DMARK}\":\"dmark\\n\""
    fi
}

set-up-fzm-bindings

Let me know what you think and I'll update the pull request #43

@urbainvaes
Copy link
Owner

Sounds great to me! I think it's fine if the new feature is available with ctrl-g only in bash >4. I will do some tests and then happy to merge. Thank you very much again for your work on this, you did a very thorough job! :)

@akinomyoga
Copy link
Collaborator

akinomyoga commented Dec 31, 2020

(sorry I mistakenly posted an incomplete comment. I deleted the incomplete comment)

Thank you!

Would you mind adding ble.sh native binding? I think this is the cleanest way when ble.sh is loaded, but it is also understandable to reject the bindings for minor plugins like ble.sh when you don't want to maintain the minor bindings.

diff --git a/fzf-marks.plugin.bash b/fzf-marks.plugin.bash
index 4af287b..c55e25e 100644
--- a/fzf-marks.plugin.bash
+++ b/fzf-marks.plugin.bash
@@ -133,7 +133,9 @@ function jump {

 function pmark {
   local selected="$(echo "${1}" | sed 's/.*: \(.*\)$/\1/' | sed "s#^~#${HOME}#")"
-  if [ "${BASH_VERSINFO[0]}" -lt 4 ]; then
+  if [[ $_ble_attached ]]; then
+      ble/widget/insert-string "$selected"
+  elif [ "${BASH_VERSINFO[0]}" -lt 4 ]; then
       printf '%q' "$selected"
   else
       READLINE_LINE="${READLINE_LINE:0:$READLINE_POINT}$selected${READLINE_LINE:$READLINE_POINT}"
@@ -164,7 +166,9 @@ function dmark {
 }

 function set-up-fzm-bindings {
-    if [ "${BASH_VERSINFO[0]}" -lt 4 ]; then
+    if [[ $BLE_VERSION ]]; then
+        ble-bind -c C-g fzm
+    elif [ "${BASH_VERSINFO[0]}" -lt 4 ]; then
         bind "\"\C-g\":\"fzm\C-m\""
     else
         local mark1='\200' mark2='\201' mark3='\202'

I think the temporary variable names TEMP_{LINE,POINT} can be changed to something like _FZF_MARKS_{LINE,POINT} to avoid possible conflicts.

@3ximus
Copy link
Contributor Author

3ximus commented Dec 31, 2020

Nice, works much better, it also updates the prompt in-place 👍
I updated my config for this, if @urbainvaes is ok with it I can push it too.

Cheers! Happy new year if that's the case 😉 !

@urbainvaes
Copy link
Owner

Looks good to me! Thanks both, and happy new year! :)

@akinomyoga
Copy link
Collaborator

OK, thank you! and happy new year! It seems the new year comes just 30 minutes later in the earliest time zone!

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