Skip to content

Conversation

@talineo
Copy link

@talineo talineo commented Mar 13, 2011

Hi Israel,

I'm pushing you my extraction/modifications of vo_sort functions.
As you may have noticed, I have forked your repository to be able to push my modifications to github and to your repository when appropriate.
Please look at it,
I will post it on the mailing list too.

See ya,
Fredd, aka Frédéric Heulin, aka Talineo.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some testing, I found that foldlevel() doesn't work as expected with VO, so I changed that line to:

if indent(a:line) != indent(nexthead)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should change that later back to foldlevel,
as I have completely rewritten the MyFoldLevel function and it works fine here ...
I'm still testing, will post it in a fold_test branch.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Mar 13, 2011, at 4:24 PM, talineo wrote:

+" Return 1: next is greater, 0 next is same, -1 next is less
+function! s:CompHead(line)
+" let l:thisline=getline(a:line)
+" let l:nextline=getline(s:NextHead(a:line))
+" if l:thisline <# l:nextline
+" return 1
+" endif
+" if l:thisline ># l:nextline
+" return -1
+" endif
+" return 0
+" XXX Israel's modif to test

  • let nexthead = NextHead(a:line)
  • let l:thisline=getline(a:line)
  • let l:nextline=getline(nexthead)
  • if foldlevel(a:line) != foldlevel(nexthead)

We should change that later back to foldlevel,
as I have completely rewritten the MyFoldLevel function and it works fine here ...
I'm still testing, will post it in a fold_test branch.

My only concern about changing it was that other functions might rely on that 'broken' behaviour of MyFoldLevel().

Israel

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know this is not the case.
But anyway rewriting MyFoldLevel will require still some work as It has to be speedy
and still work reliably and as you suggest, we will have to look for other plugins relying on foldlevel

regarding vo_sort.vim, I realise I didn't pushed the last version ...
will do that tonight if I can ;-)

@Raimondi
Copy link
Owner

Great, this is a step forward :)

Before merging, I'd like to point a couple of things.

The functions that appear before CompHead() don't look specific for sorting, we should better leave them in the ftplugin so they are always available.

Check the line note on line 113 of vo_sort.vim. I had to change that line to if indent(a:line) != indent(nexthead).

Cheers!
Israel

@talineo
Copy link
Author

talineo commented Mar 13, 2011

Yes, I would like to create a ~/.vim/autoload/vimoutliner* for that.
We could put common functions there.

@talineo
Copy link
Author

talineo commented Mar 13, 2011

I don't know If pushed away the comments about the foldlevel/indent issue by pushing the real version but please add it again ... can you change directly or do I need to push yet another version ?

@Raimondi
Copy link
Owner

On Mar 13, 2011, at 4:48 PM, talineo wrote:

I don't know If pushed away the comments about the foldlevel/indent issue by pushing the real version but please add it again ... can you change directly or do I need to push yet another version ?

#2 (comment)

I can make the changes, as well as restore the functions to the ftplugin.

Israel

@talineo
Copy link
Author

talineo commented Mar 13, 2011

you will have to remove the "s:" on the function name.
what do you think of the autoload file ? this way, we could have a unique name for function like vimoutliner#IsParent().

@Raimondi
Copy link
Owner

On Mar 13, 2011, at 4:56 PM, talineo wrote:

you will have to remove the "s:" on the function name.
what do you think of the autoload file ? this way, we could have a unique name for function like vimoutliner#IsParent().

I was thinking the same, it could free VO from depending on the filetype... but that would be a big change, everything is tied to the vo_base filetype. I'm also not sure how convenient that would be, after all, since VO is handled as a language the function on the ftplugin are avilable to all VO plugins.

We should change the names to make them begin with VO_ or Vimoutliner_ anyways, it's not good practice to have the current names.

Israel

@talineo
Copy link
Author

talineo commented Mar 13, 2011

let's talk about that on the mailing list, other people can give their point of view.
I'd like to push you my real version of vo_sort before you do anything, so we should close this one.

@Raimondi
Copy link
Owner

On Mar 13, 2011, at 5:13 PM, talineo wrote:

let's talk about that on the mailing list, other people can give their point of view.
I'd like to push you my real version of vo_sort before you do anything, so we should close this one.

#2 (comment)

vo_sort.vim doesn't work here, is it working on your end as it is?

Israel

@talineo
Copy link
Author

talineo commented Mar 14, 2011

Test the one push in my development branch. It works for me at least.
Please tell me how that is misbehaving so I can see whether
I still have the problem in the last version.

@Raimondi
Copy link
Owner

On Mar 14, 2011, at 3:23 AM, talineo wrote:

Test the one push in my development branch. It works for me at least.
Please tell me how that is misbehaving so I can see whether
I still have the problem in the last version.

I just pulled from your development branch and when I try ,,s or ,,S the command :call SortChildren(1) or with -1 is called, but nothing happens.

Israel

@Raimondi
Copy link
Owner

There is no change, the text doesn't change at all. With the debug version I get just one message ( XXX SortChildren 0 ) for ,,s and ,,S.

Since this is not finished yet, can you put the functions above CompHead() back into the ftplugin?

On other topic, I want to release 0.3.5 this weekend or the beginning of the next week and I don't think we have enough time to test this plugin, do you mind if we hold it for the release of 0.3.6-7?

@talineo
Copy link
Author

talineo commented Mar 17, 2011

no problem to hold it for the release, please send me your test file by mail please.

@Raimondi
Copy link
Owner

I did some more testing and it works. The problem was that I didn't know the behaviour had change. Before, ,,s and ,,S sorted the children of the header with the cursor, now it sorts all siblings of the header with the cursor, except on the top level.

@talineo
Copy link
Author

talineo commented Mar 18, 2011

You mean, it would have worked with

a
d
b
c

where a,b,c,d are at the root level ?
I doubt it worked because vo_sort is based on closing all the folds of the parent down to the leaves.
I may look into sorting at the root level thought as it may also be interesting.

Why not include it after all ?
I can try and add the root level sorting for next version.

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