search

sachac / subed

subed is a subtitle editor for Emacs
177 stars 16 forks source link

Finish up MELPA checklist #8

Closed riscy closed 5 years ago

riscy commented 5 years ago

These commits get this package caught up with the MELPA checklist per https://github.com/melpa/melpa/pull/6255.

I've tried to use a gentle touch, but I've opted to remove the subed-config.el file and merge its contents into subed.el -- for example, we need subed's defgroup to be declared before setting a number of variables that rely on it -- you can see that in the first commit.

The second commit fixes a bug re: sharp-quotes -- sharp-quotes should only be used when the target is a function.

The third commit is basically compiler pacifier, but in some cases you may decide that moving the function/variable out of subed.el is good idea. For example, subed--buffer-file-name is only ever used in subed-mpv.el; subed-save-excursion is only ever used in subed-srt.el, etc. The less cyclic you can make your dependency graph between these files, the happier you'll be.

The last commit is just a minor lint on some conditionals.

rndusr commented 5 years ago

I've pulled all your commits except for the subed-config.el removal which I'd like to handle differently.

Merging subed-config.el into subed.el makes it even bigger than it already is and I don't like huge files. I'd actually like to split up subed.el even further.

How about renaming subed-config.el to subed.el and move the stuff that is currently in subed.el to other files?

Are there any other reasons besides having defgroup in subed.el?

I put subed--buffer-file-name in subed.el because it has nothing to do with mpv. The mpv code is currently the only place where it's used, but subed is far from done and it could be used in other places in the future. If subed-ass.el would use functions from subed-mpv.el, that would be confusing.

Same thing regarding subed-save-excursion: It's generally useful and will likely be used by other subtitle format implementations. Advanced users can also use it when extending subed.

Thanks for mentoring me.