Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add strings module #96

Merged
merged 12 commits into from
May 30, 2021
Merged

Conversation

delphinus
Copy link
Contributor

@delphinus delphinus commented Mar 13, 2021

ref #95
ref nvim-telescope/telescope.nvim#464

TODO

  • need to transfer another funcs from telescope.nvim?
  • doc

@delphinus delphinus mentioned this pull request Mar 13, 2021
@elianiva
Copy link
Contributor

elianiva commented Mar 13, 2021

could probably add this to align multibyte string as well. The lua string.format("%-5s", some_string) freaks out when it receives a multibyte character.

https://github.com/nvim-telescope/telescope.nvim/blob/c8cc024ebf86d58dc37913e1e9ec7e465d687d68/lua/telescope/utils.lua#L334

@Conni2461
Copy link
Collaborator

Thanks for porting this :)

Do you mind also adding border title truncate?
Should happen here:

local title_len = string.len(title)

Max width is: content_win_options.width

lua/plenary/strings.lua Outdated Show resolved Hide resolved
@delphinus delphinus marked this pull request as ready for review March 15, 2021 10:14
@Conni2461
Copy link
Collaborator

Do you also wanna add this dedent function? We use it in telescope (docs) and tree-sitter-lua docgen (tests) and its easier if it would be part of plenary.

local dedent = function(str, leave_indent)
  -- find minimum common indent across lines
  local indent = nil
  for line in str:gmatch('[^\n]+') do
    local line_indent = line:match('^%s+') or ''
    if indent == nil or #line_indent < #indent then
      indent = line_indent
    end
  end
  if indent == nil or #indent == 0 then
    -- no minimum common indent
    return str
  end
  local left_indent = (' '):rep(leave_indent or 0)
  -- create a pattern for the indent
  indent = indent:gsub('%s', '[ \t]')
  -- strip it from the first line
  str = str:gsub('^'..indent, left_indent)
  -- strip it from the remaining lines
  str = str:gsub('[\n]'..indent, '\n' .. left_indent)
  return str
end

@delphinus
Copy link
Contributor Author

@Conni2461 I added dedent(). The original logic cannot deal with 'tabstop', so I rewrote all with using strdisplaywidth().

delphinus added a commit to nvim-telescope/telescope.nvim that referenced this pull request Mar 26, 2021
for line in str:gmatch('[^\n]*\n?') do
-- It matches '' for the last line.
if line == '' then
goto CONTINUE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we not use goto breaks lua 5.1 (and some people build neovim with it)

Copy link
Contributor Author

@delphinus delphinus Apr 26, 2021

Choose a reason for hiding this comment

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

Fixed on a90e9b0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied again 8931abd

@Conni2461
Copy link
Collaborator

The original logic cannot deal with 'tabstop', so I rewrote all with using strdisplaywidth()

The original was just some hacky thing tj wrote so we can write tests like this:
https://github.com/tjdevries/tree-sitter-lua/blob/ca38705ab4551db996428ebce6bed9bdc9b77780/lua/tests/docgen/renderer_spec.lua#L124-L146

Its not really used in production but thanks anyways :)

@tjdevries
Copy link
Member

One other thing as well is that if there are any string functions that exist within neovim that would be useful but just aren't exposed, we can expose them over the vim.str_* functions via the C api in neovim core. I haven't looked much at the code here yet to know if that's true (and also I don't know anything about non-ascii characters 😆 )(

@delphinus
Copy link
Contributor Author

@Conni2461
I tested the case for it. ( d46e342 ) I think the current implementaation can deal with the one you pointed. How do you think?

@tjdevries
I looked into the code for vim.str_* and it seems possible for strdisplaywidth() because the logic exists in charset.c (here). But the logic for strcharpart() exists in func.c (here) and I cannot find the method to call it from executor.c. How can I do this?

static int nlua_str_charpart(lua_State *const lstate) FUNC_ATTR_NONNULL_ALL
{
  f_strcharpart(...); // What should be here for arguments?
  return 1;
}

@tjdevries
Copy link
Member

Hey @delphinus , are you asking what code you would need to add to neovim core to be able to do this?

If so, I can guide you through the PR that you'd probably want to be able to do that.

@Conni2461
Copy link
Collaborator

@delphinus Whats the progress here? Some people are waiting for a telescope PR nvim-telescope/telescope.nvim#528 (comment) that is already done but requires the truncating border titles.

I would be fine with merging a not done version here. (for example everything besides dedent if you have trouble with removing the goto)

delphinus added a commit to delphinus/plenary.nvim that referenced this pull request Apr 26, 2021
delphinus added a commit to delphinus/plenary.nvim that referenced this pull request Apr 26, 2021
@delphinus
Copy link
Contributor Author

@tjdevries @Conni2461

Sorry for too large delay! I have been too busy to write code here, so I (or some person) will improve in the future when I have time ;( Can you merge this current code?

delphinus added a commit to delphinus/plenary.nvim that referenced this pull request Apr 26, 2021
delphinus added a commit to delphinus/plenary.nvim that referenced this pull request Apr 26, 2021
delphinus added a commit to delphinus/plenary.nvim that referenced this pull request Apr 26, 2021
lua/plenary/strings.lua Outdated Show resolved Hide resolved
delphinus added a commit to delphinus/plenary.nvim that referenced this pull request May 3, 2021
@Conni2461
Copy link
Collaborator

Somehow the goto is back. I have no idea why 🤣

Can you also rebase nvim-telescope/telescope.nvim#690 so i can test it in combination? After that i think we can merge.

Thanks :)

delphinus added a commit to nvim-telescope/telescope.nvim that referenced this pull request May 15, 2021
@delphinus
Copy link
Contributor Author

Fixed again. and nvim-telescope/telescope.nvim#690 is also ready.

@VVKot
Copy link
Contributor

VVKot commented May 25, 2021

@tjdevries @Conni2461 any chance you can take a look at this PR? As far as I can tell it is blocking nvim-telescope/telescope.nvim#528 which I quite fancy.

Copy link
Collaborator

@Conni2461 Conni2461 left a comment

Choose a reason for hiding this comment

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

This looks good to me. I ask tj if i can merge it :)

@Conni2461 Conni2461 merged commit f560613 into nvim-lua:master May 30, 2021
@maxxnino
Copy link

Some how i got error when using :Telescope find_files with this commit
NVIM v0.5.0-dev+1362-g3cd688ff7
error

@Conni2461
Copy link
Collaborator

Sorry there was a "typo" i missed. Should be fixed

@maxxnino
Copy link

Thank that fixed the problem

Conni2461 pushed a commit to nvim-telescope/telescope.nvim that referenced this pull request Jun 14, 2021
@delphinus delphinus deleted the feature/strings-utils branch May 18, 2022 00:42
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.

None yet

6 participants