weizheheng / ror.nvim

Have FUN builiding Ruby on Rails applications with Neovim!
MIT License
132 stars 15 forks source link

Notification alternatives #12

Closed otavioschwanck closed 1 year ago

otavioschwanck commented 1 year ago
image image
weizheheng commented 1 year ago

Thank you so much for the PR, this looks great. I just pushed something up just now, and it seems like it causes some conflicts. Do you mind resolving the conflicts? Can't wait to try this out on my local.

otavioschwanck commented 1 year ago

Thank you so much for the PR, this looks great. I just pushed something up just now, and it seems like it causes some conflicts. Do you mind resolving the conflicts? Can't wait to try this out on my local.

@weizheheng done

weizheheng commented 1 year ago

@otavioschwanck Nice, thank you, I actually like this more than my previous implementation of a floating window showing the result, thinking of totally replacing with this and fallback to vim.notify if user doesn't have nvim-notify install

otavioschwanck commented 1 year ago

@otavioschwanck Nice, thank you, I actually like this more than my previous implementation of a floating window showing the result, thinking of totally replacing with this and fallback to vim.notify if user doesn't have nvim-notify install

Awesome!

weizheheng commented 1 year ago

@otavioschwanck, Thank you so much for this PR, I have taken a lot of inspiration from here.

There are a few things that I am doing differently:

  1. Notification window is buffer specific
    • This means that each test file buffer will have its own nvim-notify instance.
    • The reason for doing this is that, when I want to run tests in a different tab, the position of the notification window will not be affected by the other instance.
    • Doing this also allow this plugin to clear the notification window specifically for the current test file buffer.
  2. When the test is done running, it will replace the notification window instead of creating a new one.
  3. Set the notification window timeout to false.
    • User can clear the notification window using require("ror.test").clear()
    • Rerunning the test in a test buffer will also dismiss any existing notification windows belonging to the buffer.

You can give it a try on the main branch!

otavioschwanck commented 1 year ago

@otavioschwanck, Thank you so much for this PR, I have taken a lot of inspiration from here.

There are a few things that I am doing differently:

  1. Notification window is buffer specific

    • This means that each test file buffer will have its own nvim-notify instance.
    • The reason for doing this is that, when I want to run tests in a different tab, the position of the notification window will not be affected by the other instance.
    • Doing this also allow this plugin to clear the notification window specifically for the current test file buffer.
  2. When the test is done running, it will replace the notification window instead of creating a new one.
  3. Set the notification window timeout to false.

    • User can clear the notification window using require("ror.test").clear()
    • Rerunning the test in a test buffer will also dismiss any existing notification windows belonging to the buffer.

You can give it a try on the main branch!

if i was you, i would put the timeout to be configurable, i personally dont like doing that.

weizheheng commented 1 year ago

@otavioschwanck, yeap, I can see that will be helpful. Just added the ability to customise the timeout through config by default is false

require("ror").setup({
  test = {
    notification = {
      -- Using timeout false will replace the progress notification window
      -- Otherwise, the progress and the result will be a different notification window
      timeout = false
    },
  }
})