Cycle bullets #907

robertgzr · 2020-06-06T11:18:28Z

When the syntax local cnfig cycle_bullets is set (currently only
markdown has it by default) the indent functions will cycle through the
characters defined in bullet_types based on the level of indentation.

Signed-off-by: Robert Günzler r@gnzler.io

Steps for submitting a pull request:

ALL pull requests should be made against the dev branch!
Take a look at CONTRIBUTING.MD
Reference any related issues.
Provide a description of the proposed changes.
PRs must pass Vint tests and add new Vader tests as applicable.
Make sure to update the documentation in doc/vimwiki.txt if applicable,
including the Changelog and Contributors sections.

robertgzr · 2020-06-06T11:21:19Z

opening this as draft to see if it's acceptable :)

I like using different bullet chars to indicate the indentation level of list elements, it's especially useful with long lists that flow outside the viewport of my editor.

robertgzr · 2020-06-06T11:22:47Z

syntax/vimwiki_markdown.vim

@@ -73,8 +73,9 @@ let s:markdown_syntax.rxHR = '\(^---*$\|^___*$\|^\*\*\**$\)'
 let s:markdown_syntax.rxTableSep = '|'

 " Lists
-let s:markdown_syntax.bullet_types = ['-', '*', '+']
+let s:markdown_syntax.bullet_types = ['*', '-', '+']


I guess this line change is more of a personal preference than anything else...

I do prefer , +, - : each time lighter or the contratyr -,+, : for a default it is easyer to remember

I think that makes sense, so let's go with *, +, -

I actually just tried it and just writing the test cases I find it harder to visually distinguish * and +:

* aaaaaa * bbbbbb + cccccc - dddddd + eeeeee

* aaaaaa * bbbbbb - cccccc + dddddd - eeeeee

robertgzr · 2020-06-06T11:29:24Z

hmm I didn't check this would still work on numbered lists, my bad. will have a look at that

tinmarino

Can you add more tests: what happens when inserting a new line with Go:

* lvl1
  + lvl2
    - lvl3
      * lvl4

tinmarino · 2020-06-06T12:06:50Z

I didn't check this would still work on numbered lists, my bad. will have a look at that

Add the tests to Vader please: nice to see PR with some tests become to come

opening this as draft to see if it's acceptable :)

If will be accepted when you feel ready: just ping me

robertgzr · 2020-06-06T16:49:31Z

@tinmarino can you have a look here. I added the latest neovim version to the test setup (it's what I'm using)

my manual testing and that new neovim test suite run fine, all the vim suites fail the test you asked for here

tinmarino · 2020-06-06T23:36:58Z

The error is from:

    (33/38) [ EXPECT] (X) New item
      - Expected:
          * Love
            - Very
              + Much
              +
      - Got:
          * Love
            - Very
              + Much
            -

It fails in my vader, not in my personal vim instance. The problem is that + is not considered as a bullet type in the vader configuration.

tinmarino · 2020-06-06T23:37:34Z

Not a serious error, if tired, just comment the test out.

tinmarino · 2020-06-06T23:40:26Z

autoload/vimwiki/lst.vim

@@ -1338,7 +1343,7 @@ function! vimwiki#lst#change_marker(from_line, to_line, new_mrkr, mode) abort
        else
          "if the old is ### and the new is * use ***
          if cur_item.type == 1 &&
-                \ index(vimwiki#vars#get_syntaxlocal('multiple_bullet_chars'),


Can you justify that ? I guess it is because the variable is declare in syntaxlocal so you fixed a bug.
It would be more coherent to have it in syntaxlocal as recurring_bullet so to change the varaible declaration in vars.
What do you think ?

I started this patch on master, there multiple_bullet_chars was retrieved via syntaxlocal. when rebasing on dev I noticed it was coming from wikilocal everywhere else, but you're right... it get's called through both. Should I just leave it? And we can follow this up with another PR (I can open an issue to track that)?

this is with my patch...

$ rg multiple_bullet_chars autoload/vimwiki/lst.vim 1106: \ index(vimwiki#vars#get_wikilocal('multiple_bullet_chars'), 1127: \ index(vimwiki#vars#get_wikilocal('multiple_bullet_chars'), 1333: if index(vimwiki#vars#get_wikilocal('multiple_bullet_chars'), s:first_char(new_mrkr)) > -1 1346: \ index(vimwiki#vars#get_wikilocal('multiple_bullet_chars'), 1404: \ index(vimwiki#vars#get_wikilocal('multiple_bullet_chars'),

I guess the line was just missed here: ac4d0a1?

tinmarino · 2020-06-06T23:44:24Z

Te resume:

Fix or comment test: markdown syntax, ensure + is accepted as bullet point
Multiple_bullet_chars: from wikilocal to syntaxlocal
Optional: If wanna have fun, comment some (all) function in lst.vim as I've jsut did un base.vim: it helps newcommers

Anyhow, very nice contribution. It will be accepted in any case, I just try to minimize my integration additions.

When the syntax local config `cycle_bullets` is set (currently only markdown has it by default) the indent functions will cycle through the characters defined in `bullet_types` based on the level of indentation. Signed-off-by: Robert Günzler <r@gnzler.io>

test/independent_runs/map.vader

tinmarino · 2020-06-07T23:01:19Z

Well 1. changes I requested for have been, made; 2. the PR has its own tests and 3. The optional feature do not break the former behavior.
Good job!

robertgzr · 2020-06-08T07:49:41Z

@tinmarino thanks for the help on this :) really glad to have found this

ndaman · 2020-06-11T19:57:40Z

I don't know if this is the right place for this, but I believe this change introduces an error when changing the indentation of checklists. There is now an error when I try to indent my checklist: Key not present in Dictionary: cycle_bullets.

bewuethr · 2020-06-12T19:34:09Z

@ndaman Same here; I get

E716: Key not present in Dictionary: cb !=? ''

davidsierradz · 2020-06-13T20:43:14Z

Is this feature behind a toggle? I would like to deactivate it

teranex · 2020-07-10T20:52:55Z

Is this feature behind a toggle? I would like to deactivate it

I came here looking to ask exactly the same. I really hate this new cycle behaviour. Meanwhile i found it can be disabled by editing syntax/vimwiki_markdown.vim and setting let s:markdown_syntax.recurring_bullets to 0 (at line 77), but that will have to be done after each update.

When the syntax local config `cycle_bullets` is set (currently only markdown has it by default) the indent functions will cycle through the characters defined in `bullet_types` based on the level of indentation. Signed-off-by: Robert Günzler <r@gnzler.io>

robertgzr commented Jun 6, 2020

View reviewed changes

robertgzr force-pushed the cycle-bullets branch from 201b10c to 925249e Compare June 6, 2020 11:41

tinmarino reviewed Jun 6, 2020

View reviewed changes

tinmarino self-assigned this Jun 6, 2020

robertgzr force-pushed the cycle-bullets branch 5 times, most recently from 97fb240 to 20a7695 Compare June 6, 2020 15:08

tinmarino requested changes Jun 6, 2020

View reviewed changes

robertgzr force-pushed the cycle-bullets branch from 20a7695 to d5f058f Compare June 7, 2020 12:07

robertgzr commented Jun 7, 2020

View reviewed changes

test/independent_runs/map.vader Show resolved Hide resolved

robertgzr marked this pull request as ready for review June 7, 2020 12:19

tinmarino approved these changes Jun 7, 2020

View reviewed changes

tinmarino merged commit 61093f4 into vimwiki:dev Jun 7, 2020

robertgzr deleted the cycle-bullets branch June 8, 2020 07:48

Nudin added this to Done in v2.6 Jun 8, 2020

brennen mentioned this pull request Jun 13, 2020

Bullet cycling breaks for users of VimWiki syntax #911

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cycle bullets #907

Cycle bullets #907

robertgzr commented Jun 6, 2020

robertgzr commented Jun 6, 2020

robertgzr Jun 6, 2020

tinmarino Jun 6, 2020

robertgzr Jun 6, 2020

robertgzr Jun 6, 2020

robertgzr commented Jun 6, 2020

tinmarino left a comment

tinmarino commented Jun 6, 2020

robertgzr commented Jun 6, 2020 •

edited

tinmarino commented Jun 6, 2020

tinmarino commented Jun 6, 2020

tinmarino Jun 6, 2020

robertgzr Jun 7, 2020

robertgzr Jun 7, 2020 •

edited

robertgzr Jun 7, 2020

tinmarino commented Jun 6, 2020

tinmarino commented Jun 7, 2020

robertgzr commented Jun 8, 2020

ndaman commented Jun 11, 2020

bewuethr commented Jun 12, 2020

davidsierradz commented Jun 13, 2020

teranex commented Jul 10, 2020

Cycle bullets #907

Cycle bullets #907

Conversation

robertgzr commented Jun 6, 2020

robertgzr commented Jun 6, 2020

robertgzr Jun 6, 2020

Choose a reason for hiding this comment

tinmarino Jun 6, 2020

Choose a reason for hiding this comment

robertgzr Jun 6, 2020

Choose a reason for hiding this comment

robertgzr Jun 6, 2020

Choose a reason for hiding this comment

robertgzr commented Jun 6, 2020

tinmarino left a comment

Choose a reason for hiding this comment

tinmarino commented Jun 6, 2020

robertgzr commented Jun 6, 2020 • edited

tinmarino commented Jun 6, 2020

tinmarino commented Jun 6, 2020

tinmarino Jun 6, 2020

Choose a reason for hiding this comment

robertgzr Jun 7, 2020

Choose a reason for hiding this comment

robertgzr Jun 7, 2020 • edited

Choose a reason for hiding this comment

robertgzr Jun 7, 2020

Choose a reason for hiding this comment

tinmarino commented Jun 6, 2020

tinmarino commented Jun 7, 2020

robertgzr commented Jun 8, 2020

ndaman commented Jun 11, 2020

bewuethr commented Jun 12, 2020

davidsierradz commented Jun 13, 2020

teranex commented Jul 10, 2020

robertgzr commented Jun 6, 2020 •

edited

robertgzr Jun 7, 2020 •

edited