ethanholz / freeze.nvim

A Neovim plugin for freeze
MIT License
28 stars 5 forks source link

Freeze ranges, output to custom directory, accept freeze configuration #2

Closed cloudbridgeuy closed 7 months ago

cloudbridgeuy commented 7 months ago

A very rough draft that adds some functionalities to the project:

  1. Support providing a path to a config or using user instead of the default full.
  2. Support freezing ranges.
  3. Open the last created page in macOS using open (this needs some refactoring)

BTW, I love freeze

image

ethanholz commented 7 months ago

Thanks for the PR! The current implementation of the :Freeze user command already supports using a range, is the advantage just that we can use Lua directly instead of relying on the user command?

As for passing a custom config, I am all for that but then it should not default to the ~/.config/freeze as this will already get picked up by freeze. If using a custom config, I believe we should pass this as an option and only modify the execution if the option exists.

While I personally do like the addition of using a timestamp for naming files, we might want to consider refactoring this to be configurable by the user in opts rather than requiring it.

Lastly, what formatter was used when making these changes? I would like to standardize some of that for contributions.

cloudbridgeuy commented 7 months ago

Thanks for the PR! The current implementation of the :Freeze user command already supports using a range, is the advantage just that we can use Lua directly instead of relying on the user command?

You are right! The first time I tested it, it didn't work for me, but I removed my code and re-tested it, and it does. So I must have done something wrong the first time I used it and got misled.

As for passing a custom config, I am all for that but then it should not default to the ~/.config/freeze as this will already get picked up by freeze. If using a custom config, I believe we should pass this as an option and only modify the execution if the option exists.

The path ~/.config/freeze is the default path to the dir where the screenshots will be stored. I changed it so that its default value is . instead.

Regarding the default config, I can't get it to use my user config by default. I might need to do something differently here though.

While I personally do like the addition of using a timestamp for naming files, we might want to consider refactoring this to be configurable by the user in opts rather than requiring it.

I've refactor this so that the dir, and output (previously prefix) are optional. Moreover, the output can work as a kind of template, where {timestamp}, {filename}, {start_line}, and {end_line} will be substituted at freeze-time.

This is my current configuration for the plugin:

local home = os.getenv("HOME")

return {
  {
    dir = "~/.config/nvim/lua/user/freeze",
    opts = {
      dir = home .. "/.config/freeze",
      config = "user",
      output = "freeze-{filename}-{start_line}_{end_line}-{timestamp}.png",
      open = true,
    },
  },
}
cloudbridgeuy commented 7 months ago

Oh, and for the linter I'm using stylua through conform.nvim.

ethanholz commented 7 months ago

The path ~/.config/freeze is the default path to the dir where the screenshots will be stored. I changed it so that its default value is . instead.

Ah, I see. Yeah, I much prefer the new changes to that. Everything is looking really great!

I will test throughout the next few days and give a review/merge after that!

ethanholz commented 7 months ago

So far things are looking good! Two changes, I think the default config referenced should be "base" over "full" and dir needs to have a default value ( we can just set it to ".").

Once we get those too added, I will get this merged! Thank you again for taking the time!

cloudbridgeuy commented 7 months ago

I agree with both changes.

Done!