OmniSharp / omnisharp-vim

Vim omnicompletion (intellisense) and more for C#
http://www.omnisharp.net
MIT License
1.7k stars 169 forks source link

Add configuration variable for log directory #730

Closed lrdickson closed 3 years ago

lrdickson commented 3 years ago

I am attempting to use omnisharp-vim as part of my nix configuration. However, omnisharp-vim tries writing logs to the plugin directory, which is in my nix store and is a read only directory.

This change adds a configuration variable for the log directory. If the configuration variable is left unset, the logs are written in the plugin directory as before. If the configuration variable is set, logs are written to the specified directory. If the directory doesn't exist, it gets created.

nickspoons commented 3 years ago

All of this from plugin/OmniSharp.vim can just go at the top of autoload\OmniSharp\log.vim instead:

if has('win32')
  let default_log_dir = expand('<sfile>:p:h:h:h') . '\log'
else
  let default_log_dir = expand('<sfile>:p:h:h:h') . '/log'
end

let s:logdir = get(g:, 'OmniSharp_log_dir', default_log_dir)
let s:stdiologfile = s:logdir . '/stdio.log'

" Make the log directory if it doesn't exist
if !isdirectory(g:OmniSharp_log_dir)
  call mkdir(g:OmniSharp_log_dir, 'p')
end

function! OmniSharp#log#GetLogDir() abort
  return s:logdir
endfunction

The OmniSharp#log#GetLogDir() function can then be used from OmniSharp#Install.

Sound good?

lrdickson commented 3 years ago

That sounds good to me. This is my first pull request. Is that a change that you will make or is there something you need me to do?

nickspoons commented 3 years ago

This is my first pull request.

Nice!

Is that a change that you will make or is there something you need me to do?

We can do it either way. If you want to practice, then you can do it. Otherwise I'll merge as-is and update afterwards. Let me know what you'd prefer.

lrdickson commented 3 years ago

Does this look correct now?

nickspoons commented 3 years ago

Thanks @lrdickson! Aside from that one comment, this looks good and works well. I'm going to merge and just make that one change afterwards.