conventional / cleanup changes

[grolongo]

Hi!

First of all, great job. I've been using your package for the past 2 weeks and it's working good for me.

Since you're asking for feedback and criticism let me give you a few that you should consider (these are mostly for convention since I've no complain on the general behavior of your package):

1. I'm not sure how you're loading your package from your init.el, but intuitive-tab-line.el is missing (provide 'intuitive-tab-line) on the last line in order to be loaded with use-package
2. Since your package depends on the built-in tab-line.el, you should require it at the very beginning of intuitive-tab-line.el with (require 'tab-line)
3. You should rename all my/ functions & variables you created with the name of the package instead, so for example my/shift-tab-left would become intuitive-tab-line-shift-tab-left. The use of my/ or your initials to name your functions and so on is for personal custom elisp stored in the config file of the user, to avoid name conflicts.
4. Functions and variables that are purely internal and not interactive should hold double hyphen: (setq my/current-tab-list (list (current-buffer))) becomes (setq intuitive-tab-line--current-tab-list (list (current-buffer)))
5. Instead of overwriting a built-in function of tab-line.el, you should just give it a custom name: (defun tab-line-tabs-mode-buffers ()... becomes (defun intuitive-tab-line-buffers-list ()...
6. Lastly, do not hard-code external packages function and settings, you should write a README with configuration instructions:

   
   If you use use-package, configure it like this:  

(use-package intuitive-tab-line :load-path "git/intuitive-tab-line-mode" :bind (("C-" . tab-line-switch-to-next-tab) ("M-" . intuitive-tab-line-shift-tab-left) ("M-" . intuitive-tab-line-shift-tab-right)) :custom (tab-line-tabs-function 'my/tab-line-tabs-buffers) (tab-line-switch-cycling t) :config (global-tab-line-mode 1))

I made all the changes mentioned above in the fork https://github.com/grolongo/intuitive-tab-line-mode  
Just let me know if you want me to make a pull request.

I also have another question regarding development in combination with tab-bar but I'll open a new issue later.

[thread314]

Thanks so much! It means a lot that someone other than me has found it helpful. :)

And thanks for the thoughtful feedback. I am self-taught and very much an amateur, so I have my fair share of blindspots.

In particular I notice there is a number of conventions I'm unaware of. Out of curiousity is there a convenient place where these are all laid out?

All those changes sound very sensible, a pull request would be great. :)

Regards,

On Sun, 2 Apr 2023 at 01:27, grolongo @.***> wrote:

Hi!

First of all, great job. I've been using your package for the past 2 weeks and it's working good for me.

Since you're asking for feedback and criticism let me give you a few that you should consider (these are mostly for convention since I've no complain on the general behavior of your package):

1. I'm not sure how you're loading your package from your init.el, but intuitive-tab-line.el https://github.com/thread314/intuitive-tab-line-mode/blob/master/intuitive-tab-line.el is missing (provide 'intuitive-tab-line) on the last line in order to be loaded with use-package
2. Since your package depends on the built-in tab-line.el, you should require it at the very beginning of intuitive-tab-line.el with (require 'tab-line)
3. You should rename all my/ functions & variables you created with the name of the package instead, so for example my/shift-tab-left would become intuitive-tab-line-shift-tab-left. The use of my/ or your initials to name your functions and so on is for personal custom elisp stored in the config file of the user, to avoid name conflicts.
4. Functions and variables that are purely internal and not interactive should hold double hyphen: (setq my/current-tab-list (list (current-buffer))) becomes (setq intuitive-tab-line--current-tab-list (list (current-buffer)))
5. Instead of overwriting a built-in function of tab-line.el, you should just give it a custom name: (defun tab-line-tabs-mode-buffers ()... becomes (defun intuitive-tab-line-buffers-list ()...
6. Lastly, do not hard-code external packages function and settings, you should write a README with configuration instructions:

If you use use-package, configure it like this:

(use-package intuitive-tab-line :load-path "git/intuitive-tab-line-mode" :bind (("C-" . tab-line-switch-to-next-tab) ("M-" . intuitive-tab-line-shift-tab-left) ("M-" . intuitive-tab-line-shift-tab-right)) :custom (tab-line-tabs-function 'my/tab-line-tabs-buffers) (tab-line-switch-cycling t) :config (global-tab-line-mode 1))

I made all the changes mentioned above in the fork https://github.com/grolongo/intuitive-tab-line-mode Just let me know if you want me to make a pull request.

I also have another question regarding development in combination with tab-bar but I'll open a new issue later.

— Reply to this email directly, view it on GitHub https://github.com/thread314/intuitive-tab-line-mode/issues/3, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACA6ZJS76XT2TFMBZBD2FHDW7BQR5ANCNFSM6AAAAAAWPYEOKU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[grolongo]

You're welcome! I'm self-taught as well and just trying to learn as I go :)

About the conventions, you can find the official doc here:

https://www.gnu.org/software/emacs/manual/html_node/elisp/Coding-Conventions.html
you can also find the same page directly in Emacs if you prefer.

You could have also use M-x flymake-mode to list different advices / warnings / errors directly into your file.

Lastly you can as well open a built-in library, for example M-x find-library search for abbrev, and take a look at the file overall to get a good grasp about it's structure.

I'll make the pull request then!

[thread314]

Merged. :)