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

The default purpose for buffers visiting files should be edit, not general #113

Closed
deb0ch opened this issue Nov 4, 2017 · 9 comments
Closed

Comments

@deb0ch
Copy link
Contributor

deb0ch commented Nov 4, 2017

I was adding various major modes / filename regexes to have the right purpose when editing xml files, Dockerfiles, Jenkinsfiles and whatnot and something struck me:

all buffers visiting a file are for doing some kind of edition, so every buffer visiting a file should have the edit purpose, right ?

We could just check the result of buffer-file-name before checking all the major-mode and regex rules, to set the default to edit instead of general for buffers visiting files.

Because I don't think it is doable or reasonable to keep track of all possible name regex and major mode of buffers that should be in edit purpose, right ?

I don't mind implementing it, if you confirm me first that such a feature makes sense 👷

@bmag
Copy link
Owner

bmag commented Nov 4, 2017

There was a similar report to Spacemacs, actually: syl20bnr/spacemacs#8238

The idea makes sense. If you're willing to implement it, I'll merge it. (as long as the code is fine)

@deb0ch
Copy link
Contributor Author

deb0ch commented Nov 4, 2017

Ok great, I'll get to it whenever I have a few coding hours 👍✨

@bmag
Copy link
Owner

bmag commented Nov 4, 2017

Thanks. Don't forget to include tests. I think these would be enough:

  • opening a file (find-file) that doesn't have a configured purpose should default to edit
  • opening a file that has configured purpose should use that purpose

@deb0ch
Copy link
Contributor Author

deb0ch commented Nov 4, 2017

Yep, for sure I will 👍 ✨

It was simpler than expected, I already implemented the feature.

Implementing the test might be a bit harder, I'm really not familiar with ert or automated testing in general 😕

Any advice ?

In the meantime I'm opening a PR with the current implementation, to have a first review.

deb0ch added a commit to deb0ch/emacs-purpose that referenced this issue Nov 4, 2017
@deb0ch
Copy link
Contributor Author

deb0ch commented Nov 4, 2017

Implemented the tests, but couldn't test them.

See #114 , I have to go now ! :shipit:

@deb0ch
Copy link
Contributor Author

deb0ch commented Nov 5, 2017

All clean and ready now 👍✨

@bmag
Copy link
Owner

bmag commented Nov 5, 2017

Thanks, commented on PR. About running the tests, it's quite simple:

make install
make test

Testing for a specific version of Emacs:

EMACS=/path/to/emacs make install
EMACS=/path/to/emacs make test

@deb0ch
Copy link
Contributor Author

deb0ch commented Nov 5, 2017

I don't know what I was missing but I tried again today and didn't have difficulty to install it after all.

I guess I wasn't thinking clearly because I was in a hurry 😅

deb0ch added a commit to deb0ch/emacs-purpose that referenced this issue Nov 5, 2017
@bmag bmag closed this as completed in b6e92de Nov 7, 2017
@bmag
Copy link
Owner

bmag commented Nov 7, 2017

Fixed 😄

wyuenho pushed a commit to wyuenho/emacs-purpose that referenced this issue May 27, 2018
wyuenho pushed a commit to wyuenho/emacs-purpose that referenced this issue May 31, 2018
wyuenho pushed a commit to wyuenho/emacs-purpose that referenced this issue May 31, 2018
wyuenho pushed a commit to wyuenho/emacs-purpose that referenced this issue May 31, 2018
wyuenho pushed a commit to wyuenho/emacs-purpose that referenced this issue May 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants