Workflow proposal

[leiserfg]

I don't know of a better place to write this so I'm doing it here but feel free to redirect me.

It will be nice if the groups are inherited from "default" so one can just add some augends for certain filetypes without having to duplicate the default group.
It will be also nice to make dial to use the group with the name of the filetype if it exists otherwise use 'default', that way instead of having several maps one could use the same and profit.

If you are ok with those, please, let me know. We could discuss the api and I could implement the requested changes.

By now I think both could be accomplished with this api:

 -- This one is a function I also think deal should have, probably with another name
local function words(vals)
     return augend.constant.new {
        elements = vals,
        word = true,
        cyclic = true,
        }
end

  require("dial.config").augends:register_group {
        default = {
          augend.date.alias["%Y-%m-%d"],
          ...
          },
          python = {
             words { "False", " True"  },
             extending="default" -- Or maybe true if it's from default, so less typing
            }
   }
   
   vim.keymap.set("n", "<C-A>", dm.inc_normal(filetype=true), { noremap = true })
   ...

[leiserfg]

Also, if you have a (semi)automated way of running the test, let me know how so I can run them myself.

[monaqa]

Thanks for the proposal.
The automation of testing is an area that we have not worked on, and any PR would be welcome.

The topic about register_group() function is more of an API proposal than a workflow. It might be better to change the issue title or create a separate issue.

[leiserfg]

Yes, they will be probably 2 PRs for this, but I think they make more sense when used together, that's why a "workflow proposal". In any case, if there is a place to better discuss it before starting any implementation, then just let me know.

[monaqa]

For now, the only place we have for discussion is here (issue page), so I hope we can discuss it here (or another issue).

[monaqa]

We would like to continue to discuss changes to the API regarding configuration.

The current API for register_group is intentionally kept simple. In fact, simply inheriting multiple augends can still be accomplished with code like the following:

local augends_base = {
    augend1,
    augend2,
    ...
}
require("dial.config").augends:register_group {
    default = augend_base,
    python = list_concat { augend_base, {augend3} }
}

where list_concat is a function that concatenates multiple array-like tables.

Inherits is a convenient feature for the user, but there is also a concern about the complexity of API and inner implementation. When self-references or cross-references occur, dial.nvim must detect them and raise an error. Such errors would not be detected by the Lua language server.

[leiserfg]

True, then the inheritance should be achieved by just using vim.list_extend, it makes sense not to complicate the api, but will be nice to add it to the doc as an example at least.

Regarding the file format thing, do you think it's better to extend the api as I said or to add extra functions for that use case instead?

[monaqa]

will be nice to add it to the doc as an example at least.

Agreed. I will consider adding it to the README and help.

For filetypes, I think the latter idea, i.e., add extra functions, is preferable, like:

require("dial.config").augends:on_filetype {  -- or more preferable function name
    python = list_concat {
        augend_base,
        {
            words {False, True}
        },
    markdown = list_concat {
        augend_base,
        {
            augend.misc.alias.markdown_header,
        }
    }
}

[leiserfg]

No, I mean if you are fine with dm.inc_normal(filetype=true)
Or you wanna have a different set of functions for filetypes.

[monaqa]

My understanding is that by providing a function like augends:on_filetype(), API extensions like dm.inc_normal(filetype=true) should be unnecessary.

[leiserfg]

So you mean we should make inc_normal and similars trigger both kinds of groups? or that when calling them, the group of the filetype will act as "default"?
I'm not sure if that will make it more clear, check my PR, the implementation is hacky it looks like a sql injection 😄 but the api seems clean to me (no needs of having two set of groups and no internal changes)

[monaqa]

My idea is:

1. Provide augends:on_filetype() function in dial.config module, which can be used in the following ways:

   require("dial.config").augends:on_filetype {  -- or more preferable function name
       python = concat_lists {
           augend_base,
           {
               words {False, True}
           },
       },
       markdown = concat_lists {
           augend_base,
           {
               augend.misc.alias.markdown_header,
           },
       },
   }
   

2. require("dial.map").inc_normal() behaves like:

   • If the 'filetype' of the buffer is either "python" or "markdown", then the augends registered in on_filetype are used.
   • If the 'filetype' of the buffer is neither "python" nor "markdown", then the default augends are used.

3. When the argument is not omitted, like require("dial.map").inc_normal("group_name"), it behaves as it does now.

---

And now I'm checking the second PR (#53).
First of all, thank you for your very cool idea and preference to preserve the API of dial.nvim.
However, I'd like to provide a secure API, even if the internal implementation is a little more complicated.
To be honest, I was not aware that injections were possible. The risk of abuse may be small as it is just a plugin setting, but as a developer, I feel that we do not want to allow code given as a string to be executed.

I will consider appropriate ways to implement filetype support. I would appreciate your opinions, as there are many things I am not aware of on my own.

[leiserfg]

Yes, I'm aware of the hackiness of my PR, but I think it's better to keep a similar api (obviously, more secure) like making inc_normal(name: str| () -> str| nil ) , so instead of doing a "lua string injection" the user could simply do (keeping my PR naming):

vim.keymap.set("n", "<C-A>", dm.inc_normal(require("dial.extras")._resolve_ft), { noremap = true })

Adding on_filetype makes the intention less clear and at the same time less flexible IMHO. The current implementation of inc_normal (and similars) is returning a string but the mapping could instead use a regular lua closure.

[monaqa]

I tried adding functionality based on the on_filetype idea (#54).
I do not intend to merge this PR immediately and will discuss which policy seems better before deciding to merge.
With my implementation, users will be able to set different augend rules for each filetype without changing the mapping settings at all.
I think it is clear and flexible enough, because users can still use augend:register_group() function if they desire a more complex customization.
How do you think?

[leiserfg]

Your PR is good enough for my usecase, as having per-filetype augends is what I really need. The idea of this pr was to make the thing more flexible, allowing other kinds of rules. But yes, maybe this is good enough and there is no need for that kind of flexibility.