alphapapa / makem.sh

Makefile-like script for linting and testing Emacs Lisp packages
GNU General Public License v3.0
163 stars 13 forks source link

Support .dir-locals.el #9

Open noctuid opened 4 years ago

noctuid commented 4 years ago

It would be nice if settings from .dir-locals.el were applied. elisp-lint supports this, and I need it for a couple checkers to prevent failure.

I'm trying it out and it seems to work well. nix-emacs-ci is really nice. I didn't know about github actions either. I'll have to try that too at some point. Would be nice to also be able to do CI testing on OSX. I'll see if I can get that working without having to change anything in the script.

alphapapa commented 4 years ago

It would be nice if settings from .dir-locals.el were applied. elisp-lint supports this, and I need it for a couple checkers to prevent failure.

Hm, that's an interesting idea. Is it possible to load such settings without requiring confirmation interactively for risky local vars?

I'm trying it out and it seems to work well. nix-emacs-ci is really nice. I didn't know about github actions either. I'll have to try that too at some point. Would be nice to also be able to do CI testing on OSX. I'll see if I can get that working without having to change anything in the script.

Cool, let me know!

noctuid commented 4 years ago

Hm, that's an interesting idea. Is it possible to load such settings without requiring confirmation interactively for risky local vars?

If you just want to unconditionally load everything, I think you can just override safe-local-variable-p to always return non-nil.

Cool, let me know!

I got it working on travis with OSX (for test at least, didn't try anything else, but it should work). I'll see if I can easily get it working on bsd too. Would you accept a pr that adds a section before everything else that conditionally adds a bunch of functions based on $OSTYPE? For example:

function sed {
    gsed "$@"
}
...
alphapapa commented 4 years ago

Hm, that's an interesting idea. Is it possible to load such settings without requiring confirmation interactively for risky local vars?

If you just want to unconditionally load everything, I think you can just override safe-local-variable-p to always return non-nil.

Hm, I think that would be a bad idea. Could expose vulnerabilities.

What causes dir-local variables to be applied? It looks like it's the function hack-local-variables, but if there is no open buffer in the directory, I guess it won't happen. I don't know if it would be practical or proper to try to force that before running rules. Can you give me some examples of how you need it?

I got it working on travis with OSX (for test at least, didn't try anything else, but it should work). I'll see if I can easily get it working on bsd too. Would you accept a pr that adds a section before everything else that conditionally adds a bunch of functions based on $OSTYPE? For example:

function sed {
    gsed "$@"
}
...

Probably not. I'd strongly prefer to not add platform-specific workarounds, especially for proprietary platforms. Would it be possible to do this another way, e.g. some kind of wrapper script that exported functions like that and then called makem.sh? If all of the OS-specific workarounds were contained in a single, separate file, I'd probably be willing to add that and document it for those who need to use it.

noctuid commented 4 years ago

Can you give me some examples of how you need it?

Right now, I'm setting fill-column, indent-tabs-mode, and sentence-end-double-space (heresy maybe, but I'm not going to rewrite all my docstrings immediately, and if I do, I'm not going to do it completely manually). I'll probably add more settings in the future. Maybe some way to explicitly set safe-local-variable-values would be better?

Probably not. I'd strongly prefer to not add platform-specific workarounds, especially for proprietary platforms. Would it be possible to do this another way, e.g. some kind of wrapper script that exported functions like that and then called makem.sh? If all of the OS-specific workarounds were contained in a single, separate file, I'd probably be willing to add that and document it for those who need to use it.

I'd like to use this elsewhere, so I was thinking of putting it in it's own repository. A user who wants it could then copy it into their project or add it as a submodule. I can make a PR to just add some instructions to the readme. Does that sound reasonable?

GNU getopt is not available for FreeBSD or OpenBSD as far as I can tell. There is a more compatible version of getopt that supports longopts on FreeBSD, and I was able to get makem.sh work with various options on FreeBSD. I'll try OpenBSD's longopt version of getopt next. I doubt there are that many Emacs developers on bsd, but it seems to work okay with almost the same changes as for OSX.

alphapapa commented 4 years ago

Sorry, I forgot to reply to this and it fell out of my notifications. Feel free to ping me in the future when necessary. :)

Right now, I'm setting fill-column, indent-tabs-mode, and sentence-end-double-space (heresy maybe, but I'm not going to rewrite all my docstrings immediately, and if I do, I'm not going to do it completely manually). I'll probably add more settings in the future. Maybe some way to explicitly set safe-local-variable-values would be better?

I'm thinking that the best option at the moment would probably be to add a --with-dir-locals option to makem.sh that would call hack-dir-local-variables and/or hack-dir-local-variables-non-file-buffer before testing or linting. I'm not sure of the best way to integrate that; it might need to be done for each rule that would need it, or maybe it could be done at a higher level.

Alternatively, maybe we could just find-file on each file. I don't know if that causes dir-locals to be applied in batch mode, but it might.

However, a concern is that any dir-local variables that are not safe would probably cause a prompt, which would probably make the script appear to hang. I don't know if that could be handled in a clean way. It might require a hacky workaround, like enabling -vv whenever --with-dir-locals is enabled so the user can see the prompt, but that wouldn't help when the script is being run non-interactively, like from CI.

Another possibility might be to apply specific variables with hack-one-local-variable. I'd prefer to avoid that, because my goal is to require no per-project configuration, but that might be the best way in the end.

What do you think? Thanks.

Oh, and regarding:

I'd like to use this elsewhere, so I was thinking of putting it in it's own repository. A user who wants it could then copy it into their project or add it as a submodule. I can make a PR to just add some instructions to the readme. Does that sound reasonable?

Probably, yes. As long as it's simple to use, I don't mind recommending it.

noctuid commented 4 years ago

Alternatively, maybe we could just find-file on each file. I don't know if that causes dir-locals to be applied in batch mode, but it might.

Elisp-lint does this, and it works in batch mode.

However, a concern is that any dir-local variables that are not safe would probably cause a prompt

It looks like in batch mode it just ignores all local variables if any are not safe.

What do you think?

It looks like there is an issue about safe variables on elisp-lint too (but no discussion). I think it would be adequate to allow specifying a value for safe-local-variable-values. I don't think there is a good way to avoid configuration in this case if the user has unsafe local variables that are actually needed for linting. All the variables I'm using that affect linting are safe. I'm guessing there aren't many cases where you would need to set unsafe variables for this purpose. If there's not already a way to apply safe variables only, it seems possible to do this manually. I'm guessing that would handle 99% of cases where linting needs local variables set. If the variables were for configuring an external linting package makem uses, makem could maybe have it's own safe-local-variable-values for those to avoid the need for configuration.

Probably, yes. As long as it's simple to use, I don't mind recommending it.

I created with-gnu-utils; it works with osx on CI. I can add some more instructions there. I can eventually make a PR to mention it in the readme, or if you just want to add a link to it saying makem.sh can be run through it on FreeBSD and OSX, that would be good too.

alphapapa commented 1 year ago

See also #39.