zyedidia / micro

A modern and intuitive terminal-based text editor
https://micro-editor.github.io
MIT License
24.42k stars 1.16k forks source link

Add support for MacOS crontab syntax highlighting #3172

Closed nv-pipo closed 3 months ago

nv-pipo commented 3 months ago

Description of the problem or steps to reproduce

The default configuration doesn't highlight crontab on MacOS, when running 'crontab -e'

Specifications

Version: 2.0.13 Commit hash: 68d88b57 Compiled on October 21, 2023

OS: MacOS 14.3.1 Terminal: alacritty

Solution

Create a file '~/.config/micro/syntax/crontab.yaml', and add detection for MacOS file path:

filetype: crontab

detect:
  filename: "(crontab$|[/]tmp[/]crontab[.])"
...

After that, micro is able to detect crontab on MacOS:

image

Request

For others, could you please update the default runtime/syntax/crontab.yaml file to add support for MacOS crontab paths?

JoeKar commented 3 months ago

This is not only a crontab -e or runtime/syntax/crontab.yaml symptom since it will hit every time someone tries to edit something with e.g. sudoedit xyz, because then the file will be created as xyz.[HASH] and xyz$ as regex doesn't fit any longer. So from my point of view this is a more generic use case which should be properly handled.

Maybe something xyz(\\.[\w\d]+)?$ for single files having their own rule and/or are placed at a location with usually no write access. This includes e.g. apacheconf, crontab, ini and most probably more...

nv-pipo commented 3 months ago

Hi @JoeKar,

I think there is a misunderstanding.

I'm suggesting to add [/]tmp[/]crontab[.] to the existing regex for detecting crontab files. To add support for MacOS crontab -e.

(The current regex is crontab$.)

To be more complete: Running sudoedit filename on latest ubuntu, gives the path /var/tmp/filename.XX5EFk7S.

Further checking shows that the new regex would add support for detecting sudoedit crontab:

image

While the current regex is already matching sudoedit filename.crontab:

image
JoeKar commented 3 months ago

I think there is a misunderstanding.

I don't think so. What I was really referring to was the fact that adding an explicit file path will maybe solve it for the current scenario, but what if the temporary path will be different from /tmp/?

Take just this example: grafik

I'd like to fix this more generic and already thought about file edits besides crontab, which was the reason why I started talking about some generic filenames in the /etc/ tree, because we've some other filetypes watching for some single files instead of a language (see the examples in my previous comment) where the same approach can be applied when they will be edited with sudoedit.

nv-pipo commented 3 months ago

That looks more complex than the regex for most other filetypes...but sure, it covers the crontab -e in MacOS, so it would fix this "issue"

JoeKar commented 3 months ago

That looks more complex than the regex for most other filetypes

A lot of the other file types describe whole languages and some of them a specific files. If these files are located at location the user usually can't write in the first place, then they should get this more complex rule to cover the above mentioned scenarios as well.

@dmaluka What do you think about this? Add e.g. (\\.[\w\d]+)?$ at the end of some concrete files names to cover such temporary files (e.g. started with sudoedit or crontab -e) too?

dmaluka commented 3 months ago

This is not only a crontab -e or runtime/syntax/crontab.yaml symptom since it will hit every time someone tries to edit something with e.g. sudoedit xyz, because then the file will be created as xyz.[HASH] and xyz$ as regex doesn't fit any longer. So from my point of view this is a more generic use case which should be properly handled.

Not in all cases. If the file has an extension, e.g. xyz.go, then AFAICS the temporary file will be xyz[HASH].go.

@dmaluka What do you think about this? Add e.g. (\\.[\w\d]+)?$ at the end of some concrete files names to cover such temporary files (e.g. started with sudoedit or crontab -e) too?

I think: imagine you're a cron developer, and you are opening crontab.c. Do you want it to be highlighted as a C file or as a crontab file?

Well, actually you'd be lucky and it would be highlighted as a C file, just because c.yaml is alphabetically before crontab.yaml, so it is checked first. But for example man.yaml is alphabetically after crontab.yaml, so crontab.1 is gonna be highlighted as a crontab file, not as a man page.

Well, we have support for file signature checks, which you implemented in https://github.com/zyedidia/micro/pull/2819, which should help in such ambiguous situations. And it does help... as long as we do have suitable signatures for the given pair of file types, sufficient for distinguishing between them. In this particular case, and probably in most other cases, we don't.

...All that said, I'm not entirely opposed to your idea, and I'm even having crazy thoughts about generalizing it even more (for example, I'm occasionally renaming e.g. foo.c to foo.c.old, intentionally not naming it foo.old.c to prevent the build system from compiling it if it automatically compiles all *.c files in the given directory; so it would be nice if foo.c.old was automagically highlighted as a C file as well). But as you can see, all this feels precarious.

dmaluka commented 3 months ago

If the file has an extension, e.g. xyz.go, then AFAICS the temporary file will be xyz[HASH].go.

BTW here is the relevant code: https://github.com/sudo-project/sudo/blob/main/src/sudo_edit.c#L134

And here is the relevant code deciding in which directory to create the temporary file: https://github.com/sudo-project/sudo/blob/main/src/sudo_edit.c#L63. Looks like it is created in either of /var/tmp, /usr/tmp or /tmp. All end with /tmp.

So, among the particular file types you mentioned, apacheconf and ini don't seem to be prone to this issue (since they have extensions in their filenames), so looks like we only need to focus on crontab? If so, given the above, looks like this should reliably fix the issue:

--- a/runtime/syntax/crontab.yaml
+++ b/runtime/syntax/crontab.yaml
@@ -1,7 +1,7 @@
 filetype: crontab

 detect:
-    filename: "crontab$"
+    filename: "crontab$|/tmp/crontab\\.\\w+$"
     signature: "^#.*?/etc/crontab"

 rules:
JoeKar commented 3 months ago

Ok, then for now we should go with the proposed solution from you (extended version of @nv-pipo).