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

[fish/bash/zsh]: Placeholder completions missing #422

Closed
lamchau opened this issue Oct 9, 2020 · 27 comments · Fixed by #446
Closed

[fish/bash/zsh]: Placeholder completions missing #422

lamchau opened this issue Oct 9, 2020 · 27 comments · Fixed by #446
Labels
bug Something isn't working

Comments

@lamchau
Copy link

lamchau commented Oct 9, 2020

Describe the bug
The original issue in #419 seems fixed, but I'm missing the placeholders when filling in my commands

To Reproduce
Steps to reproduce the behavior:

  1. navi (completion should contain <placeholder>
  2. Select command
  3. Notice <placeholder>: is missing

Expected behavior

  • <placeholder>: should appear
  • Number of commands should be consistent (See Additional context)

Screenshots

2.12.1

> ./navi --version
navi 2.12.1
> ./navi                                                                                                                                                                             
~ normal  < 1/32 +S
» cal                                normal calendar        cal -B <before> -A <after># after selecting `cal`
~   < 1/1 +S
»

2.10.0

> navi --version
navi 2.10.0
> navi 
~ normal  < 1/26 +S
» cal                                normal calendar        cal -B <before> -A <after># after selecting `cal`
before:   < 0/0 +S

Versions:

  • OS: macOS
  • Shell Version: fish 3.1.2, bash 5.0.18, zsh 5.7.1

Additional context

I also noticed

  • 2.12.1 shows 32 commands, completion shows 1/1
  • 2.10.0 shows 26 commands, completion shows 0/0

maybe related to the bug?

@lamchau lamchau added the bug Something isn't working label Oct 9, 2020
@welcome
Copy link

welcome bot commented Oct 9, 2020

Thanks for opening your first issue here! In case you're facing a bug, please update navi to the latest version first. Maybe the bug is already solved! :)

@denisidoro
Copy link
Owner

denisidoro commented Oct 9, 2020

Are you using skim instead of fzf?

@lamchau
Copy link
Author

lamchau commented Oct 9, 2020

nope!

> fzf --version
0.23.0 (brew)

@denisidoro
Copy link
Owner

I'm not able to reproduce the problem :(

Is there a $ before: ... line in your .cheat or is this variable free text?

@denisidoro
Copy link
Owner

Could you share your .cheat file?

@lamchau
Copy link
Author

lamchau commented Oct 9, 2020

here's my cal.cheat

% cal

# normal calendar
cal -B <before> -A <after>

# julian calendar
cal -j -B <before> -A <after>

@denisidoro
Copy link
Owner

I'm still not able to reproduce it. Do you mind running the following?

navi --version
bash --version
fish --version
uname -a

@lamchau
Copy link
Author

lamchau commented Oct 13, 2020

navi 2.12.1
GNU bash, version 5.0.18(1)-release (x86_64-apple-darwin19.5.0)
fish, version 3.1.2
Darwin laptop.local 19.6.0 Darwin Kernel Version 19.6.0: Mon Aug 31 22:12:52 PDT 2020; root:xnu-6153.141.2~1/RELEASE_X86_64 x86_64

@lamchau
Copy link
Author

lamchau commented Nov 29, 2020

still unable to see the placeholders in anything after 2.10.0, not sure how I can help here

navi 2.13.0
GNU bash, version 5.0.18(1)-release (x86_64-apple-darwin19.5.0)
fish, version 3.1.2
Darwin tracer.local 19.6.0 Darwin Kernel Version 19.6.0: Thu Oct 29 22:56:45 PDT 2020; root:xnu-6153.141.2.2~1/RELEASE_X86_64 x86_64

@denisidoro
Copy link
Owner

denisidoro commented Dec 7, 2020

I really don't know how to investigate this 😢

But I assume this is a very local problem because no one else has reported it yet (or maybe they gave up using navi?)

Are you a software developer? If so, could you try debugging this from your end to give me more input?

You just need to:

  • install cargo via rustup
  • add a bunch of dbg!("reached line 42"); in the code, specially in core.rs
  • run cargo run

Hopefully this will help me narrow the root cause down

Thanks!

@lamchau
Copy link
Author

lamchau commented Dec 8, 2020

i am! let me repro in a clean install on a parallels vm. it could be people don't use the parameterization to fill out commands very often (and use it more like cht)

i have 2.10.0 pinned (which has always worked) and not later versions but will definitely help to try to help debug this more

@denisidoro
Copy link
Owner

Ok! Please let me know if I can be of any help

@lamchau
Copy link
Author

lamchau commented Dec 21, 2020

@denisidoro let me know if this is any of use! the bug looks is maybe related to the rework around prompt_finder/prompt_with_suggestions (introduced in v2.11)? we can see from dbg! the value is there at least!

v2.10 src/cmds/core.rs

let value = if let Ok(e) = env_value {
    e
} else {
    variables
        .get_suggestion(&tags, &variable_name)
        .ok_or_else(|| anyhow!("No suggestions"))
        .and_then(|suggestion| {
            let mut new_suggestion = suggestion.clone();
            new_suggestion.0 = replace_variables_from_snippet(&new_suggestion.0, tags, variables.clone(), config)?;

            prompt_with_suggestions(variable_name, &config, &new_suggestion, interpolated_snippet.clone())
        })
        .or_else(|_| prompt_without_suggestions(variable_name, &config))?
};

v2.13 src/cmds/core.rs

let value = if let Ok(e) = env_value {
    e
} else if let Some(suggestion) = variables.get_suggestion(&tags, &variable_name) {
    let mut new_suggestion = suggestion.clone();
    new_suggestion.0 =
        replace_variables_from_snippet(&new_suggestion.0, tags, variables.clone(), config)?;
    prompt_finder(variable_name, &config, Some(&new_suggestion), variable_count)?
} else {
    prompt_finder(variable_name, &config, None, variable_count)?
};

from my simplified cal.cheat:

% cal
cal -B <before> -A <after>

the verbose step by step (with 2 placeholders)

   Compiling navi v2.13.0 (/Users/lamchau/github/navi)
    Finished dev [unoptimized + debuginfo] target(s) in 3.37s
     Running `target/debug/navi`

we can see here &variable_name does show the text i'm expecting before but ~ < 1/1 +S (0) shows that there's no placeholder

[src/cmds/core.rs:201] &variable_name = "before"
[src/cmds/core.rs:203] &env_value = Err(
    NotPresent,
)
~   < 1/1 +S (0)
»
[src/cmds/core.rs:201] &variable_name = "before"
[src/cmds/core.rs:203] &env_value = Err(
    NotPresent,
)
[src/cmds/core.rs:201] &variable_name = "after"
[src/cmds/core.rs:203] &env_value = Err(
    NotPresent,
)
~ 1  < 0/1 +S (0)
[src/cmds/core.rs:201] &variable_name = "before"
[src/cmds/core.rs:203] &env_value = Err(
    NotPresent,
)
[src/cmds/core.rs:201] &variable_name = "after"
[src/cmds/core.rs:203] &env_value = Err(
    NotPresent,
)
   November 2020         December 2020          January 2021
Su Mo Tu We Th Fr Sa  Su Mo Tu We Th Fr Sa  Su Mo Tu We Th Fr Sa
 1  2  3  4  5  6  7         1  2  3  4  5                  1  2
 8  9 10 11 12 13 14   6  7  8  9 10 11 12   3  4  5  6  7  8  9
15 16 17 18 19 20 21  13 14 15 16 17 18 19  10 11 12 13 14 15 16
22 23 24 25 26 27 28  20 21 22 23 24 25 26  17 18 19 20 21 22 23
29 30                 27 28 29 30 31        24 25 26 27 28 29 30
                                            31
`

@denisidoro
Copy link
Owner

Thanks! This is very helpful!

While I debug it, would you mind pasting the output for the following command (assuming there are no credentials):

env | grep before

@denisidoro
Copy link
Owner

Also, could you share the output for env | grep FZF?

@denisidoro
Copy link
Owner

denisidoro commented Dec 21, 2020

What if you run FZF_DEFAULT_OPTS="" navi --finder fzf? Does it behave as expected?

@lamchau
Copy link
Author

lamchau commented Dec 21, 2020

we're still is missing the placeholders with fish (but did the same with bash and zsh)

set -gx FZF_DEFAULT_OPTS ""
target/release/navi --finder fzf

here's my FZF_DEFAULT_OPTS before clearing

> echo "$FZF_DEFAULT_OPTS"
    --bind '?:toggle-preview'
    --bind 'ctrl-a:toggle-all'
    --bind 'ctrl-b:page-up'
    --bind 'ctrl-f:page-down'
    --bind '∆:preview-page-down'
    --bind '˚:preview-page-up'
    --bind 'ctrl-e:execute(echo {+} | xargs -o vim)'
    --bind 'ctrl-r:toggle-sort'
    --color=fg:#c2c2c2,bg:#000000,hl:#7ca4d3 --color=fg+:#eaeaea,bg+:#444444,hl+:#7ca4d3 --color=info:#c397d8,prompt:#eaeaea,pointer:#7ca4d3,marker:#bacb20,spinner:#c397d8,header:#e6c617
    --height=85%
    --info=inline
    --layout=reverse
    --multi
    --preview 'bat --style=numbers --color=always {} | head -500'
    --preview-window=:hidden
    --prompt='~ ' 
    --pointer='»' 
    --marker='✓'

and my FZF_DEFAULT_COMMAND=rg --files --hidden --no-ignore-vcs

Thanks! This is very helpful!

While I debug it, would you mind pasting the output for the following command (assuming there are no credentials):

env | grep before

nothing shows up here:

> env | grep before
<EMPTY>

@denisidoro
Copy link
Owner

Interesting

I don't know if you're aware, but the variable name isn't included in the prompt anymore. It's in the preview window:

Screen Shot 2020-12-21 at 16 53 15

Your FZF_DEFAULT_OPTS is hiding the preview window, that's why you don't see <before>:

However, it should have worked when you set it to the empty string 🤔

@lamchau
Copy link
Author

lamchau commented Dec 21, 2020

i ran into a new error with an empty FZF_DEFAULT_OPTS on the v2.13.0 tag, maybe i'm missing a configuration step?

╭──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ error: The subcommand 'preview-var' wasn't recognized                                                                                                                        1/9 │
│         Did you mean 'preview'?                                                                                                                                                  │
│                                                                                                                                                                                  │
│ If you believe you received this message in error, try re-running with 'navi -- preview-var'                                                                                     │
│                                                                                                                                                                                  │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯

@denisidoro
Copy link
Owner

denisidoro commented Dec 21, 2020

You're using the version you built from master (>=v2.13.0), right?

Then, internally it's calling the system-level navi (<=v2.10.0), which doesn't contain the preview-var command.

Please try to make your local build be the system-level one, at least temporarily.

Something like export PATH="/path/to/local/navi/target/release:$PATH"; navi

(I could try reusing the same binary used as entry-point but the code would become more complex, possibly a little bit slower. I may revisit that, though)

@lamchau
Copy link
Author

lamchau commented Dec 21, 2020

so having gone through a few different steps with my config, it looks like --preview-window=:hidden is the culprit here and doesn't play nicely with navi

i have this disabled by default because of large globs of code can slow down the experience

@denisidoro
Copy link
Owner

Alternativetly, you can add something like this to your .bashrc file:

alias navi='FZF_DEFAULT_OPTS="" navi'

@lamchau
Copy link
Author

lamchau commented Dec 21, 2020

i use navi as a keybinding (ctrl+g), so we'd need to modify the widget then no?

also, is there a place for us within navi to delete out the preview flag as we parse env?

@denisidoro
Copy link
Owner

denisidoro commented Dec 21, 2020

i use navi as a keybinding (ctrl+g), so we'd need to modify the widget then no?

yeah. you can run navi widget fish, edit it accordingly and source it in your .fishrc directly

@denisidoro
Copy link
Owner

also, is there a place for us within navi to delete out the preview flag as we parse env?

No, but given that navi requires this flag to be a certain value, it doesn't make sense to read it from env. I'll try to fix that in the next release.

@lamchau
Copy link
Author

lamchau commented Dec 21, 2020

thank you so much for helping me figure this out!

also, just noticed your comment 🤦‍♂️

Your FZF_DEFAULT_OPTS is hiding the preview window, that's why you don't see <before>:

@denisidoro
Copy link
Owner

Related: #447

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants