martineausimon / nvim-lilypond-suite

Neovim plugin for writing LilyPond scores, with asynchronous make, midi/mp3 player, fast syntax highlighting, "hyphenation" function, and support for LaTex and Texinfo files
GNU General Public License v3.0
125 stars 10 forks source link

Specifying a main file to compile (and open preview on, and open playback on) #3

Closed niveK77pur closed 1 year ago

niveK77pur commented 2 years ago

Hello, first of all sweet plugin! So far I used Frescobaldi for editing because of the midi player, the point-and-click and the auto-complete suggestions. This plugin basically tackles all of which kept me in Frescobaldi and I can finally let lose of these shackles and work in neovim for the entire process.


The issue

There is a slight nuisance which makes me not use any of the features beyond syntax highlighting and auto-completion. I like to split my lilypond project into multiple files, and I rarely spend time in the main file that needs to be compiled. On their own, these separate files cannot be compiled into the actual pdf — they are simply being \included into the main file.

I do need to compile that main file, but for that to work I currently need to switch to that one any time I want to compile, open the viewer, or play the midi. It would be great to somehow specify the main file to perform these operations on. That avoids switching back and forth all the time which can become quite distracting and annoying.

Frescobaldi has a nice feature for this. When you have multiple tabs open with different files, you can right click a tab and tick a box saying Always Engrave This Document (which I always do on the main file). Then anytime you want to compile the thing, it directly compiles the file on which you ticked that box instead of the currently selected one. It is a really nice feature, which is the only thing I am really missing from this plugin.

Possible ways to tackle it

I have been wondering for a while how one could incorporate this here though (functionality wise). Here are a few ideas I had.

martineausimon commented 2 years ago

Thank you for these suggestions, it is indeed an important function to add to this plugin! I have to take some time for this...

martineausimon commented 2 years ago

Hi ! I think it should work with the last update : Compile, open mp3, open pdf, only for main.ly if this file exists in working dir. You can also create a .lilyrc file in folder to define custom main file like this :

vim.g.nvls_main = "/complete/path/to/main/file.ly"

It's a bit dirty for the moment, and the code may need some improvements... tell me what you think !

niveK77pur commented 2 years ago

Works like a charm! Thank you so much for the effort, you rock!


I had a look at the code, I suppose the following function does the magic here? https://github.com/martineausimon/nvim-lilypond-suite/blob/2791c292d6550c6631f4220e4755797388af591d/lua/lilypond.lua#L29

At a glance, I personally would make it such that the main.ly and .lilyrc filenames are not hardcode, but they feel very sensible, so honestly it is absolutely fine like that. No need to worry about it!


There is another "issue" though with reading/processing the g.nvls_main variable (I did leave the boundaries of this plugin to summon it though).

Defining the variable in the .lilyrc file works like a charm! However, when I use a different plugin to set the variable (like exrc.nvim), then it sets the g.nvls_main variable, but the g.nvls_main_name and g.nvls_short variables are not set correctly.

As an example: I put the following in a .nvimrc.lua file

vim.g.nvls_main = "/complete/path/to/main/file.ly"

Then I open anotherfile.ly. I will see that the variables are not set correctly.

" gives `/complete/path/to/main/file.ly`
echo g:nvls_main
" gives `/complete/path/to/main/anotherfile`
echo g:nvls_main_name
" gives `anotherfile`
echo g:nvls_short

Moreover, if I then run :Viewer, :LilyPlayer pr :LilyCmp, then the g.nvls_main is reset to be /complete/path/to/main/anotherfile.ly. As a matter of fact, if I explicitly set all 3 variables, they are all reset to point to the current file after running either command.

Here an idea on how to tackle/fix this, I think it could render this functionality quite robust and not too tightly coupled to this plugin.

I have attempted the following, but it does not seem to work for the player and the viewer, even though the variables are correctly set after running a command for the first time. (It does work for compiling though!!)

diff --git a/lua/lilypond.lua b/lua/lilypond.lua
index 3e83c6c..ddf51a9 100644
--- a/lua/lilypond.lua
+++ b/lua/lilypond.lua
@@ -36,6 +36,10 @@ function M.DefineMainFile()
     g.nvls_main = "'" .. g.nvls_main .. "'"
     g.nvls_main_name = g.nvls_main:gsub('%.(.*)', "'")
     g.nvls_short = g.nvls_main:match('/([^/]+)$'):gsub('%.(.*)', '')
+  elseif g.nvls_main_file then
+    g.nvls_main = "'" .. g.nvls_main_file .. "'"
+    g.nvls_main_name = g.nvls_main:gsub('%.(.*)', "'")
+    g.nvls_short = g.nvls_main:match('/([^/]+)$'):gsub('%.(.*)', '')
   else
     if vim.fn.empty(vim.fn.glob(expand('%:p:h') .. '/main.ly')) == 0 then
       g.nvls_main = "'" .. expand('%:p:h') .. "/main.ly'"

Sorry for this huge comment, I just wanted to make sure everything is clear :sweat_smile:

martineausimon commented 2 years ago

Thank you for your feedback! I found this solution with .lilyrc to make it possible to open different projects in the same nvim session and keep nvls_main variable. I think I understand your problem even if I don't know exrc... I have to look at this! Please wait a bit before doing these tests, I still have to review the code (there was a problem with directory names containing dots, solved in last update)

martineausimon commented 2 years ago

I just installed exrc.nvim to understand how it works, but maybe you can help me a little... I try to find a way to add an additional variable or something as you suggested, it's a good idea.

(sorry for my dirty english !!)

Simon

martineausimon commented 2 years ago

Does something like this could work for you ?

I think it could be a solution, but correct me if I'm wrong : there is always an issue when opening differents files from differents projects in the same nvim session, and when calling a file in another directory than the working directory

in lua/lilypond.lua :

function M.DefineMainFile()
-- default : main file is current file
  g.nvls_main = expand('%:p:S')

-- if folder containing opened file contains .lilyrc, read it
-- (and overwrite vim.g.nvls_main)
  if fn.empty(fn.glob('%:p:h' .. '/.lilyrc')) == 0 then
    dofile(expand('%:p:h') .. '/.lilyrc')
    g.nvls_main = "'" .. g.nvls_main .. "'"

-- if folder containing opened file contains main.ly, main file is main.ly
  elseif fn.empty(fn.glob(expand('%:p:h') .. '/main.ly')) == 0 then
      g.nvls_main = "'" .. expand('%:p:h') .. "/main.ly'"
  end

-- if vim.g.nvls_main_file exists, define nvls_main from here :
  if g.nvls_main_file then
  g.nvls_main = g.nvls_main_file
  end

-- create sub variables from vim.g.nvls_main variable
  local name,out = g.nvls_main:gsub("%.(ly')", "'")
  if out == 0 then
    name,out = g.nvls_main:gsub("%.(ily')", "'")
  end
  g.nvls_main_name = name
  g.nvls_short = g.nvls_main_name:match('/([^/]+)$'):gsub("'", "")

end

in ftplugin/lilypond.lua :

lilyCmd('Viewer', function() 
  require('lilypond').DefineMainFile()
  -- redefine pdf variable
  b.nvls_pdf     = expand(
  "'" .. g.nvls_main_name:gsub("'", "") .. ".pdf'")
  require('nvls').viewer()
  end,
{})
lilyCmd('LilyPlayer', function() 
  require('lilypond').DefineMainFile()
  -- redefine midi & mp3 variables
  g.lilyMidiFile = expand(
    "'" .. g.nvls_main_name:gsub("'", "") .. ".midi'")
  g.lilyAudioFile = expand(
    "'" .. g.nvls_main_name:gsub("'", "") .. ".mp3'")
  require('lilypond').lilyPlayer() 
  end, 
{})
martineausimon commented 2 years ago

Hi Kevin, i've just updated the repo with vim.g.nvls_main_file variable to test it... Tell me what you think !

niveK77pur commented 2 years ago

Hi Simon! Sorry for not being very responsive! At any rate, (in my eyes) the general idea of a local vimrc (such as what exrc provides) is to have project local configurations. Say you have expandtab enabled in your config, but for one project in particular you want to (or need to) have tabs instead of spaces for indentation. Then you can make use of some sort of local vimrc or local init.lua to change that. This avoids cluttering your main vim config with project specific settings through autocommands. I hope this makes sense. In our case, you could think of it as launching vim as nvim +':let g:nvls_main_file = "Documents/folder/file.ly"' file.ly, but you place that command in a local vimrc file instead of the command line.

About local vimrc configs

Regarding your first point about $HOME/.nvimrc.lua – yes it is normal that opening nvim Documents/folder/file.ly from the home directory reads the $HOME/.nvimrc.lua config. The idea is to place these .nvimrc.lua files into the project folder, and then launch vim from that folder. I.e. you'd have a Documents/folder/.nvimrc.lua and then launch nvim file.ly from Documents/folder. So in theory there shouldn't be any .nvimrc.lua in your home directory. Hope that makes sense.

In the case of exrc, it seems to look for these local config files in the current directory only. Just for the sake of completeness: Before exrc I used a different plugin that was able to look for such a config file in parent directories. But I'd say that main takeaway here is that the local vimrc plugins somehow gives you the ability to (re-)define things in your vim config for a given project / folder. A bit more powerful than the +command option for nvim.

Opening multiple projects

Now regarding your second point, with all that said, you probably cannot (depending on where you launched vim), unless you manually set it every time you switch tabs. But let's say you have the following structure (assuming file1.ly and fileA.ly are the main files for both projects). Launching vim inside the project1 folder will read that project's .nvimrc.lua. Same if you launch it from project2 folder. But if you launch vim from the ./ folder, neither config will be read, so no main file will be specified either.

.
├── project1
│   ├── file1.ly
│   ├── file2.ly
│   ├── file3.ly
│   └── .nvimrc.lua
└── project2
    ├── fileA.ly
    ├── fileB.ly
    ├── fileC.ly
    └── .nvimrc.lua

It could nonetheless be interesting to open both / multiple projects inside of neovim at once. Here is what I am thinking: The g.nvls_main_file can accept a string (as is currently the case), or an array of strings (or you distinguish further between g.nvls_main_file and g.nvls_main_files (with an s) for this purpose). The array then specifies multiple main files. If you have file1.ly open for example, which would be one of the specified main files, you'll likely want to run the command on that file. But if you do not have one of the main files open (say fileC.ly), then you check in which directory the file is located, in order to determine which main file to run the commands on (in that case fileA.ly).

There'd be a few issues though, which in my eyes makes this more troublesome than is worth it. But the choice is yours!

  1. This functionality would need to be exclusive to .lilyrc files. Since local vimrc files are meant to be project specific, it doesn't make much sense to open both projects at the same time in vim. Even if you could, you would probably end up overwriting the variables and it still won't work with both projects open at the same time.
  2. This makes it such that you'd have to create an additional ./.lilyrc file where you write down all the main files. If the main files are not called main.ly, that means the user needs to manually add them to the array of main files. Another possibility is that main files happen to have a distinct filename pattern, which makes them clearly identifiable from other files. In that case one might consider making this more generic and programming some sort of file listing, which applies regex or lua patterns to filter out the main files.
  3. If you did actually make extensive use of local vimrc configs for some projects, then this approach will completely ignore these configs. In the case of exrc.nvim, it will only read the config if you open vim from inside the project folder. If you open vim in ./ and then open project1/file2.ly for example, it won't read the project1/.nvimrc.lua (similar concern to point 1.)

I think there exist plugins that help you (seamlessly) switch between different projects and their configurations that you specified, but I have personally never used them. Thinking about it that way, opening multiple projects might be best left to other plugins which already handle these things.

That's my take anyways, just providing some thoughts on the matter :grin:


Regarding the modifications, it works wonderful now! The code also seems much more sane! Using the .nvimrc.lua file works! Using the +'let g:nvls_main_file = "..."' nvim launch option works! I'd say it's perfect now :grin:

Maybe unintentional but the way you programmed it makes vim.g.nvls_main_file (as opposed to the vim.g.nvls_main) also work when specified in the .lilyrc file! I think it's a good idea to also read this variable from the .lilyrc for the sake of consistency. I wouldn't change anything in the code for this though, unless you want to make it more explicit.

If nvls_main_file is to stay, you might also want to add it to the README in case you forgot (unless I missed it, I can't find any mention of it yet). Saying that if this variable is defined (doesn't even really matter how), it will be used to set the main file.

I think from my point of view, this issue is perfectly resolved. Thank you so much for making it happen, you definitely have one very happy user of your plugin now :grin: Feel free to close the issue if nothing else remains on this front.

niveK77pur commented 2 years ago

I found this solution with .lilyrc to make it possible to open different projects in the same nvim session and keep nvls_main variable.

I actually overlooked that you said this which probably renders most of my comment completely useless lol. If it still works with .lilyrc files then you should just keep things as they are IMO. Let's say that nvls_main_file is there as an alternative option in case someone prefers to not use the .lilyrc files, but it comes with some drawbacks such as not supporting to open multiple projects.

martineausimon commented 2 years ago

Thanks for your reply, and for this valuable informations! I was waiting for your feedback before closing the issue, I'm thinking of leaving the DefineLilyVars() as it is :

I will complete the README, but - last question! - do you think that the variables g.nvls_main and g.nvls_main_file have a quite explicit name?

niveK77pur commented 2 years ago

Awesome, glad we were able to have this exchange then! :D

I think the .lilyrc is a nice thing to keep around though, it also avoids hunting down additional plugins for this purpose.

Regarding the variable names, I think g.nvls_main_file is perfect! g.nvls_main can seem a bit ambiguous, but if you mention it in the README like you do currently, then it's perfectly fine as well! If you ask me though, I would only "expose" g.nvls_main_file to the users, it seems to be the most explicit of the two to me.

martineausimon commented 2 years ago

Thanks! I think i'll expose g.nvls_main too : if you define g.nvls_main_file from .lilyrc, this variable will not be overwrited when opening another file from another project, and it is precisely the advantage of this option I think.

Last thing to improve now : point & click ! I think the following bash cmd will work better :

export LYEDITOR="nvr -s +:'dr %(file)s' && nvr -s +:'call cursor(%(line)s,%(char)s+1)' %(file)s"

If the current file is already open, it should change to that window instead of opening in current window.

niveK77pur commented 2 years ago

Awesome, sounds good to keep both then!

Did you try your LYEDITOR command? It does not work for me, it still opens the file in the current window. I am using Okular btw, not sure if that is causing it.

At any rate, I have fiddled around a bit and this here seems to work for Okular. It's also calling nvr only once which feels a bit cleaner to me. For zathura you'll want to replace %f, %l and %c accordingly and it should (still) work I hope.

nvr +:'dr %f | call cursor(%l,%c+1)'

Also well played for the %(char)+1, because the cursor was always be off by one character. I never thought of doing +1 there.

martineausimon commented 2 years ago

Thanks ! It works for zathura, and it's indeed a bit cleaner ! I'll update the README with the Okular command.

niveK77pur commented 2 years ago

Just 2 minor things.


You probably forgot to add quotes around the string for g.nvls_main_file https://github.com/martineausimon/nvim-lilypond-suite/blob/62d2adaa8be4fb8ea959966f11b0e2cb430482ff/README.md?plain=1#L156 Should be

vim.g.nvls_main_file = "/complete/path/to/custom/main/file.ly"

Regarding the LYEDITOR for Okular, it is not actually an environment variable that is set. It is simply a setting inside of Okular. It was a bit confusing beacuse you still had the export keyword there; just wanted to clarify that.

Here is the relevant page from the Okular documentation: https://docs.kde.org/stable5/en/okular/okular/configeditor.html For reference, inside of Okular you go Settings > Configure Okular ... and then go into the Editor pane and chose Custom Text Editor as the editor. Then you can paste the command into the Command field.

martineausimon commented 2 years ago

Ooops.. I wrote that too quickly ! Thanks ! Is it correct for Okular if I write :

Alternate custom text editor command, for Okular :

nvr +:'dr %f | call cursor(%l,%c+1)'
niveK77pur commented 2 years ago

Yes that's perfect!

martineausimon commented 1 year ago

Hi Kevin, I think I found a solution to use a single variable to define the main file, which seems more appropriate for neovim. A few days ago, I added a setup() function, and in the very latest update (I haven't modified the README yet), it's possible to configure the main file directly:

require('nvls').setup({
  lilypond = {
    mappings = {
      player = "<F3>",
      compile = "<F5>",
      open_pdf = "<F6>",
      switch_buffers = "<A-Space>",
      insert_version = "<F4>"
    },
    options = {
      pitches_language = "default",
      output = "pdf",
      main_file = "main.ly"
    },
  },
  latex = {
    mappings = {
      compile = "<F5>",
      open_pdf = "<F6>",
      lilypond_syntax = "<F3>"
    },
    options = {
      clean_logs = false
    },
  },
  player = {
    mappings = {
      quit = "q",
      play_pause = "p",
      loop = "<A-l>",
      backward = "h",
      small_backward = "<S-h>",
      forward = "l",
      small_forward = "<S-l>",
      decrease_speed = "j",
      increase_speed = "k",
      halve_speed = "<S-j>",
      double_speed = "<S-k>"
    },
    options = {
      row = "2%",
      col = "99%",
      width = "37",
      height = "1",
      border_style = "single",
      winhighlight = "Normal:Normal,FloatBorder:Normal"
    },
  },
})

This function can be put in .nvimrc.lua or in .lilyrc. Tell me what you think !

Thanks !

martineausimon commented 1 year ago

I wonder if i should add a main_folder option (by default "%:p:h"), for working with this kind of projets structure :

main.ly
├── folder/file1.ly
├── folder/file2.ly
└── folder/file3.ly
niveK77pur commented 1 year ago

Hi Simon, thanks for the great work! This setup function actually seems really cool, and fully in line with what I see most other plugins do!

So if I understand correctly, in the local vimrc file you would do something like this (to override the main file's name)?

require('nvls').setup({ lilypond.options.main_file = 'mymainfile.ly' })

I think this is actually pretty sweet!

Also a main_folder setting could be very interesting! That could also avoid having to specify the absolute path to the main file like we did previously(?). So I imagine something like this:

require('nvls').setup({
    lilypond.options.main_file = 'mymainfile.ly'
    lilypond.options.main_folder = '/complete/path/to/custom/main/'
})

Or this depending on how you program it (i.e. if you only extend the tables instead of replacing them)

require('nvls').setup({
    lilypond = {
        options = {
            main_file = 'mymainfile.ly'
            main_folder = '/complete/path/to/custom/main/'
        }
    }
})
martineausimon commented 1 year ago

Thanks for your reply ! That's it, and the last one works :

require('nvls').setup({
    lilypond = {
        options = {
            main_file = 'mymainfile.ly'
            main_folder = '/complete/path/to/custom/main/'
        }
    }
})

The only "issue" I had with this function is that it merges table with default values, not with the init.lua one, so you should rewrite the customs values (mappings & other options...) in .nvimrc.lua.

For the moment I don't know how to do otherwise.

niveK77pur commented 1 year ago

Hi I had a look at your implementation and I see why it's only merging with the default table: Your setup function is in fact re-initializing the options using only the default table (unless I am missing something here).

https://github.com/martineausimon/nvim-lilypond-suite/blob/b440201840f59fcfba8a1d95b0668dce42971873/lua/nvls.lua#L58

I have tried the following and it seems to resolve this "issue"! It's quite straightforward, I don't know if it could be made more concise but here you go!

M.setup = function(opts)
  opts = opts or {}
  if not vim.g.nvls_options then
    vim.g.nvls_options = vim.tbl_deep_extend('keep', opts, default)
  else
    vim.g.nvls_options = vim.tbl_deep_extend('keep', opts, vim.g.nvls_options)
  end
end

Basically, in the if branch I am checking if the global variable is not defined yet; If it isn't then vim.g.nvls_options would be nil. In this case I extend the options using the default table. However, it this global variable is defined already (i.e. in your init.lua), then we get into the else block where I simply extend the options with itself instead of the default table. Hope this makes sense!

niveK77pur commented 1 year ago

Actually here is a more concise version. Cheers.

M.setup = function(opts)
  opts = opts or {}
  vim.g.nvls_options = vim.tbl_deep_extend('keep', opts, vim.g.nvls_options or default)
end
martineausimon commented 1 year ago

Wow that's perfect !! Thank you !!

martineausimon commented 1 year ago

I think I'll close this issue now, and delete the vim.g.nvls_main_file option which is useless from now on, if I am not mistaken...

niveK77pur commented 1 year ago

Yes sounds good! This new setup will definitely deprecate the vim.g.nvls_main_file :grin:

niveK77pur commented 1 year ago

Hello again! I hope I am not overlooking something silly but the player, compilation and viewer don't seem to work with the new require().setup method. I have the following in my .nvimrc.lua

require('nvls').setup({
    lilypond = {
        options = {
            main_file = 'First-Layer.ly',
        }
    }
})

And when I run :lua =vim.g.nvls_options, I can see that the main_file is correctly set. However when I open otherfile.ly and run :LilyCmp, :LilyPlayer or :Viewer, it runs them on otherfile.ly instead of First-Layer.ly. I have also tried putting the full path into main_folder but that did not seem to have changed anything.

niveK77pur commented 1 year ago

It does not seem to work either when using .lilyrc instead of .nvimrc.lua. Here it actually seems to fall back to whatever I defined in my "original" setup function found in my main init.lua file (I gave it a funny name just so I can tell).

Looking at the code, I suspect the main_file variable definition is not in the right location. It is located at the top of the file, so I can imagine that it doesn't get redefined properly. https://github.com/martineausimon/nvim-lilypond-suite/blob/282a10b1fce9857ce42ad9c5b00fb1e5364fdc53/lua/lilypond.lua#L3

Putting it inside the M.DefineLilyVars() solve 2/3 of the issues. Compiling and Viewer work now! Only the player is still not getting the correct file. https://github.com/martineausimon/nvim-lilypond-suite/blob/282a10b1fce9857ce42ad9c5b00fb1e5364fdc53/lua/lilypond.lua#L29

Here the modification I made:

diff --git a/lua/lilypond.lua b/lua/lilypond.lua
index 955a1cc..e25287a 100644
--- a/lua/lilypond.lua
+++ b/lua/lilypond.lua
@@ -1,7 +1,5 @@
 local g, b, fn = vim.g, vim.b, vim.fn
 local expand = fn.expand
-local main_file = g.nvls_options.lilypond.options.main_file
-local main_folder = g.nvls_options.lilypond.options.main_folder

 local M = {}

@@ -28,6 +26,8 @@ end

 function M.DefineLilyVars()
   g.nvls_main = expand('%:p:S')
+  local main_file = g.nvls_options.lilypond.options.main_file
+  local main_folder = g.nvls_options.lilypond.options.main_folder

   if fn.empty(fn.glob(main_folder .. '/.lilyrc')) == 0 then
     dofile(expand(main_folder) .. '/.lilyrc')
niveK77pur commented 1 year ago

Okay with the following everything seems to work. I have also added the main_file to the M.lilyPlayer(). I see now why you had it outside of both functions. But I suspect the variables are not properly updated then, which lead to my initial issue. Putting it inside makes 100% sure we are grabbing the latest value. Again, I hope I wasn't overlooking something silly on my end which made all of this pointless lol

diff --git a/lua/lilypond.lua b/lua/lilypond.lua
index 955a1cc..4852be7 100644
--- a/lua/lilypond.lua
+++ b/lua/lilypond.lua
@@ -1,11 +1,11 @@
 local g, b, fn = vim.g, vim.b, vim.fn
 local expand = fn.expand
-local main_file = g.nvls_options.lilypond.options.main_file
-local main_folder = g.nvls_options.lilypond.options.main_folder

 local M = {}

 function M.lilyPlayer()
+  local main_file = g.nvls_options.lilypond.options.main_file
+  local main_folder = g.nvls_options.lilypond.options.main_folder
   if fn.empty(
     fn.glob(expand(main_folder) 
     .. '/' .. g.nvls_short .. '.midi')) == 0 then
@@ -28,6 +28,8 @@ end

 function M.DefineLilyVars()
   g.nvls_main = expand('%:p:S')
+  local main_file = g.nvls_options.lilypond.options.main_file
+  local main_folder = g.nvls_options.lilypond.options.main_folder

   if fn.empty(fn.glob(main_folder .. '/.lilyrc')) == 0 then
     dofile(expand(main_folder) .. '/.lilyrc')
martineausimon commented 1 year ago

Sorry I hadn't noticed this bug... I made the changes you suggested, it works perfectly now. Thanks a lot !!