Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decipher auto-mode-alist regexp and unbreak .yml support #114

Closed
wants to merge 1 commit into from

Conversation

Hi-Angel
Copy link
Contributor

I noticed that .yml files are not being opened in YAML mode. Further research showed almost unreadable regular expression, which apparently supposed to support .yml files, but failed to do so. Digging into the history to see what exactly extensions were supposed to be supported by this regexp showed:

  • raml — commit c4fd940
  • eyaml — commit 11df403
  • yml, yaml — prior to above commits

So use rx macro to enlist aforementioned extensions explicitly, and unbreak support for yml at the same time.

CC: @rhoml @scop

@Hi-Angel
Copy link
Contributor Author

Oh, turns out it's being tested, I'm wondering why yml though wasn't working then. Okay, judging by tests there's also unmentioned eyml, let me add it.

@Hi-Angel Hi-Angel force-pushed the support-yml-extension branch from 5f94cb3 to c35a9ff Compare November 24, 2024 14:18
I noticed that `.yml` files are not being opened in YAML mode. Further
research showed almost unreadable regular expression, which apparently
supposed to support `.yml` files, but failed to do so. Digging into
the history to see what exactly extensions were supposed to be
supported by this regexp showed:

* raml  — commit c4fd940
* eyaml, eyml — commit 11df403
* yml, yaml — prior to above commits

So use `rx` macro to enlist aforementioned extensions explicitly, and
unbreak support for `yml` at the same time.
@Hi-Angel Hi-Angel force-pushed the support-yml-extension branch from c35a9ff to c5f3b4a Compare November 24, 2024 14:32
@Hi-Angel
Copy link
Contributor Author

lol, I was wondering why my addition of eyml didn't fix the tests. Turned out, I "fixed it" on .el file opened from ELPA and not in the repo. Okay, now it's passing.

@syohex
Copy link
Collaborator

syohex commented Nov 24, 2024

I noticed that .yml files are not being opened in YAML mode.

I'm afraid this is not correct. The regexp matches against .yml files. You can check it by writing some s-expressions like, (string-match-p "\\.\\(e?ya?\\|ra\\)ml\\'" "foo.yml") . This expression returns non-nil value, this means the regexp matches against "foo.yml". Or you can check it by emacs -Q -l yaml-mode.el test.yml

Please check if you use the latest version. If it doesn't fix your problem, please check your configuration files.

BTW I think emacs developers are familiar with elisp regexp so we don't need to use rx macros for such simple regexps. It is not easy to read elisp regexp with default face configurations, so I recommend changing those face configurations.

(set-face-foreground 'font-lock-regexp-grouping-backslash "brightmagenta")
(set-face-foreground 'font-lock-regexp-grouping-construct "brightgreen")

@syohex syohex closed this Nov 24, 2024
@Hi-Angel
Copy link
Contributor Author

BTW I think emacs developers are familiar with elisp regexp so we don't need to use rx macros for such simple regexps. It is not easy to read elisp regexp with default face configurations, so I recommend changing those face configurations.

I feel like this is already a lost battle… but don't you think regexp optimization is better left to rx macro, because it turns human readable list of extensions to a unreadable regexp just as you like. I think most people are not masterminds who can decompile such regular expression to a list of meaningless words "yml", "eyml", etc.

@Hi-Angel
Copy link
Contributor Author

think most people are not masterminds who can decompile such regular expression to a list of meaningless words "yml", "eyml", etc

Don't get me wrong: I can do that given enough time. Like, spending ½ an hour… But why bother if you can have the list right away 🤷‍♂️

@Hi-Angel
Copy link
Contributor Author

Btw, if you consider this regexp so readable, why did you add a test that makes sure that the mode gets enabled for all extensions. I mean, it's hard to introduce a bug to this string, right?

Repository owner locked as too heated and limited conversation to collaborators Nov 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants