Skip to content

Conversation

@jakubbortlik
Copy link
Collaborator

@jakubbortlik jakubbortlik commented Nov 4, 2024

This PR adds more emojis and makes the picker configurable in the following way:

  1. The user can choose to show the emoji and/or the short name of the emoji in the picker. This makes it possible to also use the short name in the search (maybe this could even be changed to the default behaviour)
  2. The user can use a custom function for modifying how the emoji is displayed in the picker - I've spent quite a lot of time investigating why my neovim configuration does not render some emojis correctly (it misaligns skin tones, variation selectors, messes up flag emojis etc. so that there remain rendering artifacts on the screen making it difficult to pick the right emoji) and I've come to a conclusion that it's by no means a trivial question, it's often a combination of issues on the side of the editor, the terminal, terminal multiplexer (tmux), possibly also the font. gitlab.nvim should definitely not be fixing this mess, but I believe that adding a configurable formatting function is an acceptable price for helping users fix some of the problems (before those are fixed in the proper place, the editor, terminal, tmux,...).

@harrisoncramer
Copy link
Owner

Sorry @jakubbortlik think I triggered a conflict for you here

@jakubbortlik
Copy link
Collaborator Author

Sorry @jakubbortlik think I triggered a conflict for you here

No problem, I'll fix it.

This commit makes it possible to also show and search for the short names of emojis in the picker.
Also, it makes it possible for users to fix some rendering issues stemming from conflicts in how
emojis are handled in the editor/terminal/font/tmux.
@harrisoncramer
Copy link
Owner

harrisoncramer commented Nov 5, 2024

Thanks for your research into this.

  1. Here's a slimmed down version of your emojis file, it's ~50% of the size since I removed all the unnecessary fields and compacted the data, it's attached to this comment. Please swap it into your MR, to reduce the plugin size: emojis.json

  2. In the interest of keeping things simple by default, it makes more sense to just have a default behavior (show the emoji and the shortname) and then an optional callback that the user can supply which is passed the entire emoji object, if they want to do formatting themselves, like this:

    format_item = function(val)
      if state.settings.emoji.formatter then
        return state.settings.emoji.formatter(val)
      end
      return string.format("%s %s", val.moji, val.name)
    end,

This will not be used by 99% of users so I'd like to simplify it.

@harrisoncramer harrisoncramer self-assigned this Nov 5, 2024
@jakubbortlik
Copy link
Collaborator Author

2. In the interest of keeping things simple by default, it makes more sense to just have a default behavior (show the emoji and the shortname) and then an optional callback that the user can supply which is passed the entire emoji object, if they want to do formatting themselves, like this:
    format_item = function(val)
      if state.settings.emoji.formatter then
        return state.settings.emoji.formatter(val)
      end
      return string.format("%s %s", val.moji, val.name)
    end,

This will not be used by 99% of users so I'd like to simplify it.

This makes much more sense.

I've used type(state.settings.emojis.formatter) == "function" to be on the safe side.

@harrisoncramer harrisoncramer merged commit a0d10a4 into harrisoncramer:develop Nov 6, 2024
6 checks passed
@jakubbortlik jakubbortlik deleted the fix-add-more-emojis branch November 7, 2024 08:41
@harrisoncramer harrisoncramer mentioned this pull request Nov 12, 2024
harrisoncramer added a commit that referenced this pull request Nov 12, 2024
fix: Show non-resolvable notes in winbar (#417)
fix: add more emojis and make emoji picker configurable (#414)
fix: comment creation should not be possible for renamed and moved files (#416)
fix: color highlight groups are invalid (#421)
fix: plugin failing to build on Windows (#419)

---------

Co-authored-by: Jakub F. Bortlík <jakub.bortlik@proton.me>
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