Thread: [cream] modelines
Cream is a free, easy-to-use configuration of the Vim text editor
Brought to you by:
digitect
From: Damien C. <mip...@us...> - 2007-05-12 10:09:10
|
Hi there, i've been told to subscribe to talk about modelines problem, so here i am :) The modeline i use at the end of C files is : /* vim: set noexpandtab tabstop=4 softtabstop=4 shiftwidth=4: */ If the default tabstop and/or softtabstop settings in cream are differents (for example with value 8) then my modelines settings are not used. I have Cream 0.38 and Vim 7.0.133 cheers, Damien |
From: Steve H. <dig...@da...> - 2007-05-12 13:01:01
|
On Sat, 2007-05-12 at 12:09 +0200, Damien Couderc wrote: > Hi there, > i've been told to subscribe to talk about modelines problem, so here > i am :) Thanks, it's easier to discuss here than SourceForge's bug tracker. :) > The modeline i use at the end of C files is : > /* vim: set noexpandtab tabstop=4 softtabstop=4 shiftwidth=4: */ > > If the default tabstop and/or softtabstop settings in cream are > differents (for example with value 8) then my modelines settings are > not used. This is a known bug, the problem is that autocommands override modelines and Vim does not have a way (that I know) to re-read them. I just asked on the vim list, but here's a few workarounds: 1. Make your settings match your modelines. :) 2. Each time you enter the buffer, enter: :filetype detect Huge hack, but part of filetype detection is to read the modelines and by entering it at the commandline, you bypass Cream's doings. 3. Create an autocmd to do #2 in your cream-user: autocmd BufEnter * filetype detect Commands in cream-user are run last and will override Cream. 4. Write a vimscript command that can re-read/execute modelines. Vim does this by C code (see "do_modelines()" in buffer.c), but it wouldn't be to difficult to do this in vimscript. 5. C gurus only (that's you, right?): Add a function to Vim to re-read modelines, i.e., "remodeline()". I just updated CVS with statusline improvements that will display Vim's actual settings instead of Cream's globals in preparation for your patch. :) -- Steve Hall [ digitect dancingpaper com ] :: Cream... usability for Vim :: http://cream.sourceforge.net |
From: Damien C. <mip...@us...> - 2007-05-12 14:09:20
|
On Sat, 12 May 2007 09:00:57 -0400 Steve Hall <dig...@da...> wrote: > On Sat, 2007-05-12 at 12:09 +0200, Damien Couderc wrote: > > Hi there, > > i've been told to subscribe to talk about modelines problem, so > > here i am :) > Thanks, it's easier to discuss here than SourceForge's bug > tracker. :) No problem, i'm just used to limit the number of mailing lists i'm on :) > > The modeline i use at the end of C files is : > > /* vim: set noexpandtab tabstop=4 softtabstop=4 shiftwidth=4: */ > > > > If the default tabstop and/or softtabstop settings in cream are > > differents (for example with value 8) then my modelines settings > > are not used. > > This is a known bug, the problem is that autocommands override > modelines and Vim does not have a way (that I know) to re-read > them. I just asked on the vim list, but here's a few workarounds: Well i have no clue about the internals of Vim so i don't think i could help without intensive learning. If the matter is a lacking feature of Vim then maybe i can do a bit of lobbying to make things going faster. BTW if i use modelines, it is mainly because i edit different types of file that do not share the same tabulation settings (makefile 8, C files 4, etc). Regards, Damien |
From: Steve H. <dig...@da...> - 2007-05-12 20:43:31
|
On Sat, 2007-05-12 at 16:09 +0200, Damien Couderc wrote: > On Sat, 12 May 2007 09:00:57 -0400 Steve Hall wrote: > > On Sat, 2007-05-12 at 12:09 +0200, Damien Couderc wrote: > > > > > > If the default tabstop and/or softtabstop settings in cream are > > > differents (for example with value 8) then my modelines settings > > > are not used. > > > > This is a known bug, the problem is that autocommands override > > modelines and Vim does not have a way (that I know) to re-read > > them. > > Well i have no clue about the internals of Vim so i don't think i > could help without intensive learning. If the matter is a lacking > feature of Vim then maybe i can do a bit of lobbying to make things > going faster. Other than a true bug, lobbying through code is about the only way to bring things about on the Vim list. There are far too many feature requests to be implemented, even its pay-to-vote system is overburdened. > BTW if i use modelines, it is mainly because i edit different types > of file that do not share the same tabulation settings (makefile 8, > C files 4, etc). Makes perfect sense and also why I use them. Unfortunately, without a Vim feature or script to do re-read them, I don't know how to fix the conflict in Cream. -- Steve Hall [ digitect dancingpaper com ] :: Cream... usability for Vim :: http://cream.sourceforge.net |
From: Damien C. <mip...@us...> - 2007-05-17 15:16:13
|
Steve Hall <dig...@da...> wrote: > Other than a true bug, lobbying through code is about the only way > to bring things about on the Vim list. There are far too many > feature requests to be implemented, even its pay-to-vote system is > overburdened. Then i'm afraid i can't help as i don't even have enough time to work on all of my own projects. > Makes perfect sense and also why I use them. Unfortunately, without > a Vim feature or script to do re-read them, I don't know how to fix > the conflict in Cream. Sad news for modelines users. Anyway keep up the good work ! Cheers, Damien |
From: Thomas de G. de L. <deg...@ea...> - 2007-05-17 17:35:17
Attachments:
cream-modelines.patch
|
Hi Steve, Ciaran, et al., Ciaran, CCing you just to let you know, since the following is about some code of yours. The archive of this thread is here: http://sourceforge.net/mailarchive/forum.php?thread_name=20070512120913.1ff8ebec.mipsator%40users.sourceforge.net&forum_name=cream-general On 2007/05/12, Steve Hall <dig...@da...> wrote: > 4. Write a vimscript command that can re-read/execute modelines. Vim > does this by C code (see "do_modelines()" in buffer.c), but it > wouldn't be to difficult to do this in vimscript. It happens that Ciaran McCreesh has wrote such vimscript code recently, to avoid some security issues of the builtin Vim modelines system: http://ciaranm.org/show_post/120 http://ciaranm.org/show_post/128 http://ciaranm.org/files/securemodelines.vim Isn't that fortunate? :-) It integrates very well in Cream: all i had to do (see attached patch) is to copy the file in the cream directory, source it from cream.vim, and change the autocommand conditions. I've set it to re-read modelines on: - BufEnter, so that it actually overrides the Cream settings which would otherwise be re-applied, - BufWrite, so that changes one has just made to a modeline are immediatly taken into account. As you will see (that's the point of Ciaran's code), this script filters the modeline to only allow a set of safe options. I've not changed the default whitelist (it's doable in cream-user.vim anyway), but maybe you could extend it a bit (list/nolist and wrap/nowrap may be candidates for addition imho). > I just updated CVS with statusline improvements that will display > Vim's actual settings instead of Cream's globals in preparation for > your patch. :) I have made a few more modifications (again in the attached patch): - check &list instead of g:LIST, - added a &readonly test for the file status marker. I'm not sure this chunk is right, since i'm not sure i've really understood the logic of this marker (is "-" for writable or for readonly? and what is the most important, showing that the file is readonly, or that it has been edited?), but anyway, at least it makes something happen when a modeline set "ro", - rewrote the Cream_statusline_tabstop() function: your version was still paying to much attention to the g:CREAM_* globals imho, it was missleading for instance when g:CREAM_SOFTTABSTOP was set but some options were overloaded by the modeline. Mine only takes Vim options into account, which is imho simpler/safer. Note that it extends the format a bit, since 3 numbers may be displayed in weird cases: "ts[:sts[:sw]]", with sw being only shown if not equal to sts or ts, and sts only shown if not equal to ts, or if sw must be shown. Hope that helps, -- TGL. |
From: Thomas de G. de L. <deg...@ea...> - 2007-05-17 18:56:00
Attachments:
securemodelines-20070513-fix_getline.patch
|
> http://ciaranm.org/files/securemodelines.vim Found a tiny bug there: the script tries to read the last 5 lines of the buffer using getline("$-5", "$"), which doesn't work because "$-5" is not correctly interpreted (vim-7.1 here, and line("$-5") == line("$")). Thus only the last line is actually read, and modelines like this one are not found: ... /* * vim: set ... */<EOF> Attached patch fixes that. -- TGL. |
From: Ciaran M. <ci...@ci...> - 2007-05-18 08:35:42
Attachments:
signature.asc
|
On Thu, 17 May 2007 20:55:33 +0200 Thomas de Grenier de Latour <deg...@ea...> wrote: > Found a tiny bug there: the script tries to read the last 5 lines of > the buffer using getline("$-5", "$"), which doesn't work because "$-5" > is not correctly interpreted (vim-7.1 here, and line("$-5") =3D=3D > line("$")). Thus only the last line is actually read, and modelines > like this one are not found:=20 Well spotted, thanks. I've done a new release with this fix and a few other things (I handled it slightly differently to avoid parsing lines twice). On Thu, 17 May 2007 20:53:00 -0400 Steve Hall <dig...@da...> wrote: > Ciaran, can I get your permission to include your securemodelines.vim > in Cream? Sure. I've explicitly marked the new version as being redistributable under the same terms as Vim itself. On Fri, 18 May 2007 07:36:42 +0200 Thomas de Grenier de Latour <deg...@ea...> wrote: > Ciaran, would you mind adding something like this to your script?: > fun! Securemodelines_DoModelines() abort > call <SID>DoModelines() > endfun Done. Regards, --=20 Ciaran McCreesh |
From: Steve H. <dig...@da...> - 2007-05-18 02:02:41
|
On Thu, 2007-05-17 at 19:34 +0200, Thomas de Grenier de Latour wrote: > On 2007/05/12, Steve Hall <dig...@da...> wrote: > > 4. Write a vimscript command that can re-read/execute modelines. Vim > > does this by C code (see "do_modelines()" in buffer.c), but it > > wouldn't be to difficult to do this in vimscript. > > It happens that Ciaran McCreesh has wrote such vimscript code recently, > to avoid some security issues of the builtin Vim modelines system: > http://ciaranm.org/show_post/120 > http://ciaranm.org/show_post/128 > http://ciaranm.org/files/securemodelines.vim > Isn't that fortunate? :-) Very! > It integrates very well in Cream: all i had to do (see attached > patch) is to copy the file in the cream directory, source it from > cream.vim, and change the autocommand conditions. I've set it to > re-read modelines on: > - BufEnter, so that it actually overrides the Cream settings which > would otherwise be re-applied, > - BufWrite, so that changes one has just made to a modeline are > immediatly taken into account. > > As you will see (that's the point of Ciaran's code), this script > filters the modeline to only allow a set of safe options. I've not > changed the default whitelist (it's doable in cream-user.vim anyway), > but maybe you could extend it a bit (list/nolist and wrap/nowrap may > be candidates for addition imho). I'm going to lean on the original work plus your patches. We want to avoid having Cream's version fork or start missing important updates. I've asked the author for permission to include in Cream, do you think you could design the changes Cream needs so that they would be in future downloads of the original file? > > I just updated CVS with statusline improvements that will display > > Vim's actual settings instead of Cream's globals in preparation > > for your patch. :) > > I have made a few more modifications (again in the attached patch): > - check &list instead of g:LIST, Looks ok. > - added a &readonly test for the file status marker. I'm not sure > this chunk is right, since i'm not sure i've really understood the > logic of this marker (is "-" for writable or for readonly? and what > is the most important, showing that the file is readonly, or that it > has been edited?), but anyway, at least it makes something happen > when a modeline set "ro", The &buftype test should catch all readonly files, but new buffers are unwritable while not readonly. I also seem to recall that we discovered &readonly actually checks the file, a laborious task over slow networks, compounded by the frequent checking done by having it in the statusline. (But I might be wrong on that.) > - rewrote the Cream_statusline_tabstop() function: your version was > still paying to much attention to the g:CREAM_* globals imho, it was > missleading for instance when g:CREAM_SOFTTABSTOP was set but some > options were overloaded by the modeline. Mine only takes Vim options > into account, which is imho simpler/safer. Note that it extends the > format a bit, since 3 numbers may be displayed in weird cases: > "ts[:sts[:sw]]", with sw being only shown if not equal to sts or ts, > and sts only shown if not equal to ts, or if sw must be shown. I agree with your logic and tweaked it to read: tabs:4:sts8:sw8 just to make clear what is happening. As soon as Ciaran gives permission, this will all show up in CVS and make Cream 0.39. Thanks for the help, I appreciate it. -- Steve Hall [ digitect dancingpaper com ] :: Cream... usability for Vim :: http://cream.sourceforge.net |
From: Thomas de G. de L. <deg...@ea...> - 2007-05-18 05:12:17
|
On 2007/05/17, Steve Hall <dig...@da...> wrote: > I've asked the author for permission to include in Cream Oops, good catch, i had completly forgotten to check whether there was a license on this script... Sorry about that Ciaran, i know licenses are not something one should overlook and assume matching his needs, and still that's obviously what i've done. -- TGL. |
From: Thomas de G. de L. <deg...@ea...> - 2007-05-18 05:37:02
|
On 2007/05/17, Steve Hall <dig...@da...> wrote: > do you think you could design the changes Cream needs so that they > would be in future downloads of the original file? The only needed change for Cream (if Ciaran applies the tiny bugfix i've sent separatly) is on the autocmd definition. I don't think it's desirable in the original script though, since in a "normal" Vim session there is no need to re-execute modelines on BufEnter (BufWrite might still be handy, but that doesn't change the point). A solution would be to have a global function, so that we can re-define the autocmd group out of the original script. Ciaran, would you mind adding something like this to your script?: fun! Securemodelines_DoModelines() abort call <SID>DoModelines() endfun If we get such a global entry point, then all you'll have to do in Cream is to put something like this after the script is sourced: aug SecureModeLines au! au BufEnter * :call Securemodelines_DoModelines() au BufWrite * :call Securemodelines_DoModelines() aug END -- TGL. |
From: Thomas de G. de L. <deg...@ea...> - 2007-05-18 07:07:44
|
On 2007/05/17, Steve Hall <dig...@da...> wrote: > > - added a &readonly test for the file status marker. > > The &buftype test should catch all readonly files Here (gvim-7.1), if a file is writable on the filesystem but has a modeline with "set readonly", then: - &readonly == 1 (obvious), - &buftype remains empty. Hence the addition of the &readonly check. > I also seem to recall that we discovered &readonly actually checks > the file, a laborious task over slow networks, compounded by the > frequent checking done by having it in the statusline. (But I might > be wrong on that.) I can't find evidence of such a behavior here (gvim-7.1 / Linux). I have used two tabs (one with a file, and one empty), and checked the straces produced when switching from the empty one to the one with the opened file. The only thing in the status line code which seems to have an effect on the strace is the "filewritable(b:cream_pathfilename)" test: it produces 2 "access(W_OK)" calls and 4 "stat64()" calls. Other than that, the strace is always verbose (1 "open(O_RDONLY)" call immediatly followed by a "close()", and 17 other "stat64()" calls). But completly dropping the status line doesn't change that at all, so the issue seems to be elsewhere. Now, it may be different in Windows, i don't know. (Oh, and the same test in a vanilla GVim only produce one "stat64()" call, so yes, there is probably room for improvement somewhere...) > tabs:4:sts8:sw8 Nice, good idea. -- TGL. |