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

refactor: standardize fb_action user IO #72

Closed

Conversation

jamestrew
Copy link
Collaborator

Using vim.notify for output messages (log level = WARN for areas indicating missteps by users), vim.ui.input for inputs.

All messages prefixed by [telescope] to be clear about the origins of the message.

Checks off an item from #3

Using vim.notify for output messages (log level = WARN for areas
indicating missteps by users), vim.ui.input for inputs.

All messages prefixed by `[telescope]` to be clear about the origins of
the message.

Checks off an item from nvim-telescope#3
Copy link
Member

@kkharji kkharji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome refactor

return msg
end

-- Move this and above functions into telescope core?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

@fdschmidt93
Copy link
Member

fdschmidt93 commented Jan 27, 2022

Sorry, a bit too lazy right now as I've had worked on tons on the file browser in the weeks before. I'll try to get to have a first brief crack at this either tomorrow or Sunday :)

E: And of course thanks for your PR :)

@jamestrew jamestrew marked this pull request as draft January 29, 2022 20:39
@jamestrew
Copy link
Collaborator Author

Throwing this PR into draft mode to do some clean up and find a solution to #87, #91

@jamestrew
Copy link
Collaborator Author

jamestrew commented Jan 31, 2022

closes #91

I've spent some more time looking for solutions for #87 but I'm able to find something that works consistently.
So far, I'm using vim.api.nvim_command("norm :esc<CR>") and it works for all actions expect remove (due to things being printed prior to the [y/N] input getter). Also for get_valid_path if the user completely deletes the default input, it will also print the Operation aborted msg on the same line as the input prompt. There's a lot of strange behaviors I'm running into between vim.ui.input and print/vim.notify... pretty frustrating.

If anybody has any suggestions, that'll be great.

@jamestrew jamestrew marked this pull request as ready for review January 31, 2022 04:08
@fdschmidt93
Copy link
Member

I've now looked a bit more at this as I'll eventually will take this over. Couple of notes:

  • vim.ui.input is async for most users that eg user dressing.nvim or telescope-ui-select; hence, we cannot rely returning anything from vim.ui.input
  • vim.cmd [[redraw!]] is probably the most reliant way to avoid hit-enter prompt

I'll try to work incrementally and push the PR maybe in a week or two.

Thanks again anyways :)

@jamestrew jamestrew deleted the refactor/action-user-io branch March 24, 2022 22:34
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.

4 participants