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

Fix: Treat REPL buffer filetype as filetype for repl #390

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

frere-jacques
Copy link

Currently many commands only work if the cursor is on the repl buffer or on a code buffer with fitting filetype.
This is inconvenient, because one can't call IronHide, if focus is on repl and one can't call IronFocus, if cursor is on repl etc.

This fix checks if the current buffer is a repl buffer and returns the filetype that the repl was created for.

See also #323 (comment) for example.

…r code buffer, by taking repl ft as buffer ft
@noprompt
Copy link

noprompt commented Sep 13, 2024

I think it would be handy to have a user provided function in configuration instead of hard coding the filetype this way. In the referenced comment the person mentioned using iron as the filetype. I also use this but am planning to switch to iron-<filetype> for more control. (If you look at my code here you can see how I am setting the filetype.)

Maybe the config could look something like this:

{
  config = {
    repl_filetype = function(repl_ft)
      -- ...
    end
  }
}

In this way, iron can define a default implementation return value (like nil or "iron"), and people can override that if they want to. What do you think?

@frere-jacques
Copy link
Author

frere-jacques commented Sep 13, 2024

I think that might make the goal to get hide and focus working stable regardles of the current cursor more complicated.

Also what is the help of having a filetype iron? How do you work with it? I think it's quite logical to treat a window with a python interpreter as type python. If you set it to iron-python, how do you use that?

But I am not aware of the plans and ideas for this plugin. Only think I notice is thats a horrible user experience currently.

But it seems there is not much activity here anyway.

@noprompt
Copy link

noprompt commented Sep 13, 2024

Also what is the help of having a filetype iron?

In my case I am using the filetype "iron" so that I can integrate iron repl buffers with edgy.

I think it's quite logical to treat a window with a python interpreter as type python.

I disagree and here's why:

  1. Syntactically, the repl buffer contents are a super set of the filetype associated with it. I cannot substitute the contents of an arbitrary python buffer with an arbitrary ipython repl buffer.
  2. Semantically, the buffer contents are not the same. A file for code to be interpreted or compiled has a different meaning from files which contain the contents of an interactive programming session.

Thus, from a substitution perspective, I think it is illogical to treat the windows the same. The structure and meaning of the two are different.


My whole point of recommending a user extensible hook to the set the filetype is to avoid exactly this sort of unproductive discussion. 😄 By having a hook

  1. you can get what you want,
  2. others can get what they want,
  3. the maintainer doesn't have to support a concrete decision (and that is a good thing).

Everyone wins!

@frere-jacques
Copy link
Author

I am really not experienced with lua and nvim plugins. So my solution might not be the best. And I am totally open to it getting rejected.

My only strong opinion on this topic is that the main commands, like focus, hide, restart and so on should simply work, no matter if the curso is in the repl or in a buffer with the filetype.
As it is in the current commit, you get lots of errors,have to move around etc. This is in my view a bad user experience.

@noprompt
Copy link

My only strong opinion on this topic is that the main commands, like focus, hide, restart and so on should simply work, no matter if the curso is in the repl or in a buffer with the filetype.

I definitely agree!

I am not a pro lua/neovim hacker either, but I think a configuration hook is relatively easy to pull off in this case.

First, we need to extend configuration table here with a default implementation of repl_filetype which will take the repl buffer bufnr and the filetype ft corresponding to the repl (ie. "python").

{
  repl_filetype = function (bufnr, ft)
    -- Returning `nil` because that is consistent with the current behavior
    -- making this a non-breaking change.
    return nil
  end
}

In core.lua we can add a call to this function in new_repl.create_on_new_window before it returns here:

new_repl.create_on_new_window = function(ft)
  -- ...
  -- Set the filetype if configured by the user.
  local filetype = config.repl_filetype(bufnr, ft)
  if filetype ~= nil then
    vim.api.nvim_set_option_value("filetype", filetype, { buf = bufnr })
  end
  
  return meta
end

And that's it!

In my configuration, I now have:

{
          repl_filetype = function(bufnr, ft)
            return "iron"
          end,
}

Your configuration (and I tested this) would look like:

{
          repl_filetype = function(bufnr, ft)
            return ft
          end,
}

Try it out! You'll notice that the hide/show commands do, in fact, work as you'd like them to. 😄

Feel free to consider this a collaboration and use the code for your patch. 👍

@frere-jacques
Copy link
Author

Setting rhe filetype on buffer creation is indeed maybe more flexible.

In the moment I don't really want to work on that. First I want to see if there is any feedback from any maintainer in the end.

@noprompt
Copy link

noprompt commented Sep 13, 2024

I think there are actually two problems here.

  1. People want to set the filetype of the repl buffer.
  2. The key bindings you'd like to work in the repl buffer do not.

The patch I have proposed above address the first problem directly and I think it's worthy as a standalone patch for the reasons I've laid out.

The second problem, can be solved via my proposal but only if the repl_filetype is set to match the associated filetype for the repl. IOW, it's not a solution for your problem. 😄

My suggestion would be to use the code I've suggested here to solve the filetype issue or I can open a separate PR (and name you as a collaborator in the commit message so you get credit). A separate ticket/patch should be opened for your actual problem.

@frere-jacques
Copy link
Author

I am fine with that.

I only recently started to use Iron and directly noted some bugs or for me missing features. Only after I made my few contributions I read that this project was short before being archived. At the moment my fork works for me, but I hope there will be some activity here again.

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