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

Remove default keymaps and make cmd additions optional #36

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

derekbunch
Copy link

@derekbunch derekbunch commented Feb 5, 2025

while configuring this plugin, i noticed a few issues that didnt play nice with my config:

  1. there are forced keymaps that conflict with my keymaps and keymaps included by default in LazyVim
  • i prefer to define my own keymaps in keys or have a config option to accept the default keymaps, but given that the readme suggests adding them to keys and then also force sets them in the plugin, i opted to just remove the forced keymaps
  1. there were flags being passed into the dbt command at the beginning that were breaking my ability to use the plugin since my company uses a wrapper around dbt
  • for this i added some config options to enable or disable these flags, and if included, moved them to the end of the command as they are flags so position does not matter
  1. I added a configurable post cmd args that help me to be able to add flags to my dbt execution given the precommands are captured by the wrapper

  2. i attempted to generalize the _get_package_name function (although i didnt see anywhere where it is used) as it seemed very specific to the creators environment, but i can remove this if it causes any issues, just seemed like something that wouldnt really work for anyone else so tried to make it more globally useful

Copy link
Owner

@PedramNavid PedramNavid left a comment

Choose a reason for hiding this comment

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

Just a couple questions before we can merge, but thanks for the thoughtful PR!


if get_dbt_version() ~= nil and get_dbt_version() >= 1.5 then
log.debug "dbt version >= 1.5, using --log-level=INFO"
Copy link
Owner

Choose a reason for hiding this comment

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

This change will break for dbt <1.5

Copy link
Author

Choose a reason for hiding this comment

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

i thought all conditions of the statement would need to be true for it to be included?

if include_log_level and get_dbt_version() ~= nil and get_dbt_version() >= 1.5 then

if dbt_version < 1.5, wouldn't this not pass and log level not be applied even if include_log_level=true?

return _find_project_dir()
end
end
local name = vim.fn.system [[grep -E 'name: \S+' ]] + dbt_project_dir()
Copy link
Owner

Choose a reason for hiding this comment

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

wow, found an old bug thats been here for a while

Copy link
Owner

@PedramNavid PedramNavid left a comment

Choose a reason for hiding this comment

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

thanks for your contribution!

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.

2 participants