[RFC] - Extended Marks #5031

timeyyy · 2016-07-10T22:48:07Z

These are marks designed for developers who want to build plugins or embed neovim. (they follow all column and line changes)
issue #4816

In app visual testing can be done with

" Extended marks

function! LoadExtmarks()
  highlight extmark ctermbg=Blue guibg=Blue
  let g:mark_ns = nvim_create_namespace('myplugin')
  function! Testextmark(timer_id)
    " Get all the mark ids
    let a:all_marks = nvim_buf_get_marks(0, g:mark_ns, -1, -1, -1, 0)

    call clearmatches()

    for mark in a:all_marks
      let a:bytes = col([mark[1], mark[2]])
      let a:ma = matchaddpos('extmark', [[mark[1], mark[2]]])
    endfor
    call timer_start(100, 'Testextmark')
  endfunction
  call timer_start(1, 'Testextmark')
endfunction

nnoremap <leader>tm :call LoadExtmarks()<cr>

ZyX-I · 2016-07-10T22:51:32Z

src/nvim/eval.c

+{
+  int fnum = 0;
+  buf_T *buf;
+  if (argvars[0].v_type != VAR_STRING){


You should not check VAR_STRING, use one of get_tv_string* functions.

ZyX-I · 2016-07-10T23:04:38Z

test/functional/api/arbmark_spec.lua

@@ -0,0 +1,18 @@
+-- Sanity checks for buffer_* API calls via msgpack-rpc


api/ is for RPC calls or events, this should probably be directly in functional/. Also new tests are normally written with one variable per line, not local clear, nvim, buffer = …. And I would suggest to never use nvim, curbuf, curwin and avoid eval: nvim('meth', …) is meths.meth(…), same for curbuf and curwin (curbufmeths, curwinmeths), nvim('eval', 'arbmark_index(haha)') is funcs.arbmark_index('haha') (and note that you are trying to refer to variable haha in your code, I guess you actually wanted string 'haha').

Tests shouldn't live in functional/ top level, how about functional/viml.

(dict_notifications_spec.lua should be moved to functional/viml ...)

I have no idea what is supposed to be placed into functional/viml. dict_notifications_spec should be probably in eval. viml/function_spec should be in eval (and I have no idea what api_info is doing in this file, I would place it separately because function_spec is too generic; this is fine if you are testing generic function-related features like MAX_FUNC_ARGS handling, but this is no place for testing specific functions), as well as viml/errorlist_spec: other functions are tested there. Not sure about viml/lang_spec and viml/completion_spec: first may be in eval, second is such a feature that can have its own directory.

Moving viml/* to eval/ is fine with me. I think they were created by coincidence long ago.

ZyX-I · 2016-07-11T01:05:33Z

src/nvim/editor/arbmark.c

+static int pos_eq(pos_T *pos, pos_T *pos2);
+
+/* Create or update an arbmark, */
+int arbmark_set(buf_T *buf, char_u *name, pos_T *pos)


BTW, this ought to be char *. char_u * is deprecated, for strings you should use char *, for sequence of numbers uint8_t * (though mostly need to cast individual characters to uint8_t when appropriate). And, I guess, this should be const char *.

bfredl · 2016-07-11T16:48:34Z

src/nvim/api/mark_extended.c

+
+// TODO start with current buffer
+// TODO use a lru cache,
+buf_T *extmark_buf_from_fnum(int fnum)


after #4934 find_buffer_by_handle can be used for this

ZyX-I · 2016-07-12T16:54:01Z

src/nvim/buffer_defs.h

@@ -33,6 +33,7 @@ typedef struct file_buffer buf_T; // Forward declaration

 #define MODIFIABLE(buf) (!buf->terminal && buf->b_p_ma)

+


Too many blank lines.

bfredl · 2016-07-16T20:32:38Z

src/nvim/api/buffer.c

+/// @param name The mark's name
+/// @param[out] err Details of an error that may have occurred
+/// @return The (row, col) tuple
+ArrayOf(Integer, 2) mark_index(Buffer buffer, String name, Error *err)


Please prefix api functions in buffer.c with buffer_ (Or, as I expect #4934 to be merged before this, you can just use nvim_buf_ upfront, but then you will need to write tests with helpers.request for the moment until #4934 gets merged)

bfredl · 2016-07-17T09:54:32Z

Also, while out of scope of this PR, some time later I would imagine all api functions that take a line number or a position (line,col) pair to be polymorphic and also take

the name of existing marks 'a and named positions like the cursor position .
the name of an extended mark
the numerical id of an unnamed extended mark*

* I would imagine many plugins have no reason to name their private marks and would be forced to come up with dummy myfancyplugin0/myfancyplugin1 etc names. Ideally they will be able to pass the empty string to mark_set and get a unique numerical ID in return which could be passed in everywhere the name of an extended mark is expected.

It would also be useful to reuse the binary tree defined here for other kind of marks, like bufhl, manual folds etc. As I said likely out of scope of this PR (which could focus on delivering a useful MVP), but I'm pointing it out here as it might (or might not) influence the design.

timeyyy · 2016-07-17T09:59:44Z

At the moment I am following the api for tk. Text tags have more functionality than text marks. Theyvprivide the functionalit you mentioned and more. The reason youbwould use tags for this is because they live in there own namespace, you can get a tag range for a specific tag type, while with marks you are limited to the one namespace. If later on one wanted ranges of different types of marks you have to switch to the tag api or handle some mapping yourself. That might be a reason the api doesnt exist, if people want I can add the range functionality to marks as well.

timeyyy · 2016-07-19T08:15:52Z

I was thinking to introduce a namespace for the marks. This would mean bufhl and tags/ plugins create one mark that needs to be moved per position.

when doing mark_next / mark_names etc,
The results could also be filtered namespaces as well.

If we did not do this, queryibg marks will always give all marks, and not just those that a particular user of the api cares about.

bfredl · 2016-07-19T08:29:01Z

Yes, that would be very useful. That would in fact improve performance of "numerical marks". an rplugin could then increment a private counter itself, and doadd_extmark("myFancyPlugin",0,...), add_extmark("myFancyPlugin",1,...) etc without needing to wait for it to return an unique id.

Shougo · 2019-11-06T23:55:07Z

runtime/doc/api.txt

@@ -1449,6 +1479,22 @@ nvim_select_popupmenu_item({item}, {insert}, {finish}, {opts})
 nvim__inspect_cell({grid}, {row}, {col})                *nvim__inspect_cell()*
                TODO: Documentation

+                                                        *nvim_init_mark_ns()*
+nvim_init_mark_ns({namespace})


The function does not exists.
And nvim_create_namespace() API is already added in neovim.

api.txt is autogenerated. we can just delete this file from the PR.

Shougo · 2019-11-06T23:55:24Z

runtime/doc/api.txt

+
+
+                                                     *nvim_mark_get_ns_ids()*
+nvim_mark_get_ns_ids()


nvim_buf_get_extmarks()?

Shougo · 2019-11-06T23:56:48Z

Hi, I have restarted deoppet.nvim development and I will test the branch.

I think the documentation should be updated.
Because API name seems changed.

bfredl · 2019-11-09T11:53:27Z

PR thread is too long, lets continue in #11356.

ZyX-I reviewed Jul 10, 2016
View reviewed changes

marvim added the WIP label Jul 10, 2016

ZyX-I reviewed Jul 10, 2016
View reviewed changes

mhinz mentioned this pull request Jul 10, 2016

extended marks #4816

Closed

ZyX-I reviewed Jul 11, 2016
View reviewed changes

timeyyy force-pushed the extended_marks branch from 50e4778 to 2b9f23a Compare July 11, 2016 14:17

timeyyy changed the title ~~[WIP] arbitrary marks,~~ [WIP] Extended Marks Jul 11, 2016

bfredl reviewed Jul 11, 2016
View reviewed changes

timeyyy force-pushed the extended_marks branch from 2f1f6f1 to 09eaf67 Compare July 12, 2016 13:25

ZyX-I reviewed Jul 12, 2016
View reviewed changes

ZyX-I mentioned this pull request Jul 12, 2016

Provide a way to set title of existing quickfix/location list #5056

Closed

timeyyy force-pushed the extended_marks branch from a3cfb8e to 57c47bf Compare July 16, 2016 14:06

bfredl reviewed Jul 16, 2016
View reviewed changes

timeyyy force-pushed the extended_marks branch from 5d6b1e1 to 47743ae Compare July 19, 2016 12:02

timeyyy force-pushed the extended_marks branch 8 times, most recently from 1628f48 to 5da842b Compare July 11, 2019 18:22

timeyyy force-pushed the extended_marks branch 2 times, most recently from 1b62e0b to 0feacb1 Compare July 20, 2019 10:57

timeyyy force-pushed the extended_marks branch 2 times, most recently from 39ba2fc to e63bbab Compare August 3, 2019 13:44

timeyyy force-pushed the extended_marks branch 2 times, most recently from b4f8622 to ac13873 Compare August 14, 2019 16:18

timeyyy changed the title ~~Extended Marks~~ [RFC] - Extended Marks Aug 26, 2019

marvim added RFC and removed WIP labels Aug 26, 2019

timeyyy added 3 commits October 24, 2019 13:12

namespace: add ns_initialized func

a4fcdd5

nsmarks: initial commit

8b1b527

nsmarks: fix rebase conflict

276786f

timeyyy force-pushed the extended_marks branch from ac13873 to 276786f Compare October 24, 2019 11:14

bfredl added this to the 0.5 milestone Oct 28, 2019

fmoralesc mentioned this pull request Oct 29, 2019

Replace RegEx based syntax with external AST parser vim-pandoc/vim-pandoc-syntax#300

Open

3 tasks

Shougo reviewed Nov 6, 2019

View reviewed changes

runtime/doc/api.txt

*nvim_mark_get_ns_ids()*

nvim_mark_get_ns_ids()

Copy link

Contributor

Shougo Nov 6, 2019

Choose a reason for hiding this comment

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

nvim_buf_get_extmarks()?

bfredl mentioned this pull request Nov 9, 2019

Extended marks 2: Electric Boogalo #11356

Merged

bfredl closed this Nov 9, 2019

neovim locked as resolved and limited conversation to collaborators Nov 9, 2019

		@@ -0,0 +1,18 @@
		-- Sanity checks for buffer_* API calls via msgpack-rpc

		@@ -33,6 +33,7 @@ typedef struct file_buffer buf_T; // Forward declaration

		#define MODIFIABLE(buf) (!buf->terminal && buf->b_p_ma)

Uh oh!

[RFC] - Extended Marks #5031

[RFC] - Extended Marks #5031

Uh oh!

Conversation

timeyyy commented Jul 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bfredl commented Jul 17, 2016

Uh oh!

timeyyy commented Jul 17, 2016

Uh oh!

timeyyy commented Jul 19, 2016

Uh oh!

bfredl commented Jul 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shougo commented Nov 6, 2019

Uh oh!

bfredl commented Nov 9, 2019

Uh oh!

Uh oh!

timeyyy commented Jul 10, 2016 •

edited

Loading