magit / magit

It's Magit! A Git Porcelain inside Emacs.
https://magit.vc
GNU General Public License v3.0
6.44k stars 804 forks source link

Add support for custom keywords similar to `squash!` / `fixup!` #5084

Closed happy-barney closed 5 months ago

happy-barney commented 5 months ago

to support project/user specific markings.

For example:

kyleam commented 5 months ago

Thanks for PR. I see why this is useful for your given workflow/conventions, but in my view this falls under the same category of custom highlighting discussed in gh-4027. For widely used conventions, a dedicated package could make sense, and for more tailored things you can also do something like this in your config:

(advice-add
 'magit-log-propertize-keywords :filter-return
 (lambda (msg)
   (when (and (not (get-text-property 0 'font-lock-face msg))
              (string-match "\\`[a-zA-Z_][-a-zA-Z_]+[a-zA-Z_]! " msg))
     (let ((beg (match-beginning 0))
           (end (match-end 0)))
       (put-text-property beg end 'face 'font-lock-warning-face
                          msg)
       (put-text-property beg end 'font-lock-face 'font-lock-warning-face
                          msg)))
   msg)
 '((name . "my/magit-log-propertize-custom-keywords")))
happy-barney commented 5 months ago

Thanks for comment, you are right, there can by more rules applicable.

On other hand I think this problem is quite common so basic support should be in magit as well (even if not enabled by default).

What about:

kyleam commented 5 months ago

On other hand I think this problem is quite common so basic support should be in magit as well (even if not enabled by default).

I don't know how common the desire is to customize this, especially outside of some common styles/conventions that could be handled by dedicated packages (such as gh-4027).

create alist (regex => face) evaluated instead of series of when (evaluated until first match) ?

In my view this isn't worth adding and maintaining a customization knob for. If a user wants something tailored, I think the advice approach is fine for this case, and there's also magit-log-format-message-function (see gh-3384).

add support for hooks into magit-log-propertize-keywords (evaluated until first match) ?

In response to gh-3384, magit-log-format-message-function was introduced. Unless there's a strong reason that won't work, I'd prefer to use existing functionality instead of adding more pieces.

In the case of a dedicated package implementing highlighting for a custom style/convention, magit-log-format-message-function would likely be the starting point, but it may be that we need to fill in some gaps or make other adjustments on Magit's side.

kyleam commented 5 months ago

Closing for the reasons given above, but please feel free to continue the conversation here or start a discussion about alternative approaches.

happy-barney commented 5 months ago

Got your arguments, now I'm thinking how to do it, thanks for ideas :-)