Lindydancer / lisp-extra-font-lock

Highlight bound variables and quoted expressions in lisp
56 stars 5 forks source link

FYI: highlight-function-calls #3

Open alphapapa opened 7 years ago

alphapapa commented 7 years ago

Hi there,

I found your package today, but it didn't quite do what I was looking for, so I came up with this:

https://github.com/alphapapa/highlight-function-calls

Maybe you will find it useful, or maybe you can integrate it into this package! :)

Thanks.

Lindydancer commented 7 years ago

Hi!

Thanks for mailing me, it's always interesting to see what other people are doing with syntax highlighting!

When I wrote the `lisp-extra-font-lock' package I was thinking about making it highlight all function calls. I decided against it, as I was afraid it would highlight too much -- maybe I was wrong...

I was looking at your code and I noticed your comment about (0 nil)', that it shouldn't be needed. I agree -- it should not. In fact, I removed it and the package appears to work (at least when I applied it to the highlight-function-calls.el' source file itself).

And one other small thing (unless you don't want feedback, in which case you don't have to read). When you define a face, try to find a predefined face to inherit from, instead of defining it from scratch, as it makes it much easier for users to apply custom themes. In your case, highlight-function-calls--not-face' could inherit from font-lock-warning-face'. For highlight-function-calls--face' it's not as straight forward asfont-lock-function-name-face' is used to highlight the name of the current function. You could pick font-lock-preprocessor-face' as it's not used for anything in emacs-lisp mode. If you want to make a minimal change, you can inherit from theunderline' face, as it would allow users who don't like underlines to customize only one face.

-- Anders

On Thu, Aug 10, 2017 at 2:00 AM, alphapapa notifications@github.com wrote:

Hi there,

I found your package today, but it didn't quite do what I was looking for, so I came up with this:

https://github.com/alphapapa/highlight-function-calls

Maybe you will find it useful, or maybe you can integrate it into this package! :)

Thanks.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Lindydancer/lisp-extra-font-lock/issues/3, or mute the thread https://github.com/notifications/unsubscribe-auth/ADBjQM3WShkkLPIwGh9fYLgDBHqwtu3cks5sWkgGgaJpZM4OyyMs .

alphapapa commented 7 years ago

Hey, thanks, I do appreciate the feedback. I'm always looking for ways to improve my code, and I appreciate your taking the time to review it and give me advice. I am an elisp noob compared to people like you, so your lessons are valuable to me.

That's a really good point about inheriting the warning face. For myself, I think I prefer to have it a different color than the warning face, but inheriting from that would make it easier for users, and would integrate with themes. Of course, Emacs makes it so easy to apply customizations that we can have our cake and eat it too.

And thanks for the pointer on the preprocessor face, I wouldn't have thought to look for that.

Aaaand I just realized that you have packages to help with developing highlighting packages. I think I came across them a while back but forgot about them. Might have saved me some time! :)

Lindydancer commented 7 years ago

Hi!

Hey, thanks, I do appreciate the feedback. I'm always looking for ways to improve my code, and I appreciate your taking the time to review it and give me advice. I am an elisp noob compared to people like you, so your lessons are valuable to me.

You could have fooled me. Your code doesn't look like it's written by a beginner.

I try to read a lot of code -- both to pick up new tricks, but I also try to help out, if I can.

That's a really good point about inheriting the warning face. For myself, I

think I prefer to have it a different color than the warning face, but inheriting from that would make it easier for users, and would integrate with themes. Of course, Emacs makes it so easy to apply customizations that we can have our cake and eat it too.

I just realized that there is another face that might be used, `font-lock-negation-char-face'. On one hand, it's designed for cases like this. On the other hand, the default setting is the same as the default face, so for most people nothing is highlighted.

Aaaand I just realized that you have packages to help with developing

highlighting packages. I think I came across them a while back but forgot about them. Might have saved me some time! :)

Yeah, I just got too frustrated writing highlighting packages without tools. Instead of waiting for someone else to write them, I decided to write what I wanted -- it has taken a couple of years, but now I have a good collection of tools in place, most importantly a debugger and a testing tool.

If you have questions about the tools or about writing highlighting packages in general, feel free to ask.

-- Anders
alphapapa commented 7 years ago

You could have fooled me. Your code doesn't look like it's written by a beginner.

Gee, thanks. :)

I try to read a lot of code -- both to pick up new tricks, but I also try to help out, if I can.

If you have time to drop by the MELPA pull request list now and then, I'm sure Steve and syohex and submitters would appreciate your reviewing packages there. :)

I just realized that there is another face that might be used, `font-lock-negation-char-face'. On one hand, it's designed for cases like this. On the other hand, the default setting is the same as the default face, so for most people nothing is highlighted.

Thanks for pointing this out! My Solarized theme themes this, so I guess some other themes do too, so I switched to that one.

Yeah, I just got too frustrated writing highlighting packages without tools. Instead of waiting for someone else to write them, I decided to write what I wanted -- it has taken a couple of years, but now I have a good collection of tools in place, most importantly a debugger and a testing tool.

Have you thought about submitting them to Emacs itself? I feel like these tools are languishing in obscurity. :) At least, there need to be some blog posts about them.

I think I will add them to https://github.com/alphapapa/emacs-package-dev-handbook

Lindydancer commented 7 years ago

I try to read a lot of code -- both to pick up new tricks, but I also try to help out, if I can.

If you have time to drop by the MELPA pull request list now and then, I'm sure Steve and syohex and submitters would appreciate your reviewing packages there. :)

It sounds like something that I would enjoy doing. Unfortunately, time is currently my limiting factor. My family situation doesn't allow me much spare time right now. Hopefully, it might ease off in a couple of years. :-/

Yeah, I just got too frustrated writing highlighting packages without

tools. Instead of waiting for someone else to write them, I decided to write what I wanted -- it has taken a couple of years, but now I have a good collection of tools in place, most importantly a debugger and a testing tool.

Have you thought about submitting them to Emacs itself? I feel like these tools are languishing in obscurity. :) At least, there needs to be some blog posts about them.

For sure, I would not mind getting more exposure. The tools have gotten some attention, but not as much as I had hoped. For example, I thought e2ansi (a package that makes the command line tools 'more' and 'less' display files with syntax highlighting) would be something that would appeal to a many people, but only a handful seems to be using it. I've been thinking about blogging, but currently I haven't got the time to get the snowball rolling.

However, I'm not sure if you have noticed, but I have some packages in Emacs already: follow-mode, cwarn-mode, and auto-revert-mode. I have thought about submitting some packages. In fact, a couple of years ago I submitted "faceup" (my font-lock testing framework) as an attempt to promote font-lock tests for the builtin modes. Unfortunately, it was met with silence.

Also, someone suggested that I should move my tools to ELPA -- however, from my point of view, I'm happy to keep them in GitHub and MELPA for the time being.

I think I will add them to https://github.com/alphapapa/e

macs-package-dev-handbook

Thank you very much -- I'm honoured to me mentioned in the same context as the others in the list.

-- Anders
alphapapa commented 7 years ago

It sounds like something that I would enjoy doing. Unfortunately, time is currently my limiting factor. My family situation doesn't allow me much spare time right now. Hopefully, it might ease off in a couple of years. :-/

I understand. Time is always our scarcest resource.

e2ansi looks very cool! I will have to try it out. I wonder if Kaushal was inspired by it to make https://github.com/kaushalmodi/eless...

However, I'm not sure if you have noticed, but I have some packages in Emacs already: follow-mode, cwarn-mode, and auto-revert-mode.

Ah, I've been using auto-revert-mode for years! I didn't realize it was you. I think once packages are absorbed into Emacs, their authorship tends to...dilute? :) Thank you for making that package. It always works as it should, and I completely rely on it. I will check out the others.

I have thought about submitting some packages. In fact, a couple of years ago I submitted "faceup" (my font-lock testing framework) as an attempt to promote font-lock tests for the builtin modes. Unfortunately, it was met with silence.

That's too bad. Maybe people were just busy and overlooked it. I imagine that if you have time to submit it again, they will be glad to consider it, as all the current maintainers seem to do quality work and are friendly and receptive. I think they'd recognize the value of a comprehensive testing framework for a complex system like that.

Thank you very much -- I'm honoured to me mentioned in the same context as the others in the list.

There are a lot of people like you in the community, who have done a lot of great work that hasn't been noticed as much as some other packages. I hope that, a little bit at a time, I can catalog them. Please feel free to let me know if you come across any others that you think should be added.