Skip to content

Proposed fix for #1 - chmod +w buffer-file#5

Open
mplscorwin wants to merge 3 commits into
anachronic:masterfrom
mplscorwin:patch-3
Open

Proposed fix for #1 - chmod +w buffer-file#5
mplscorwin wants to merge 3 commits into
anachronic:masterfrom
mplscorwin:patch-3

Conversation

@mplscorwin
Copy link
Copy Markdown
Collaborator

First pass at fix for #1 starting with the obvious brute force approach. Expose the mode modifier and in inhibit flag to customize, predicate mode change on file being read-only, restore prior mode after if we've changed it prior. Uses before and after save hooks.

Starting with the obvious brute force approach.  Expose the mode modifier and in inhibit flag to customize, predicate mode change on file being read-only, restore prior mode after if we've changed it prior.  Uses before and after save hooks.
@mplscorwin
Copy link
Copy Markdown
Collaborator Author

Ok, that's better. Here's first blush at the most obvious brute force solution. I'm going to need to get a bit better with git in general and magic in specific I can see. Cutting and pasting from emacs into github is not the way to do this I think. A thing to consider could be `write-file-functions' instead of before-save-hook which would let's us abort the save by returning nil vs throwing the error fi the save isn't going to work after trying chmod.

addition testing target
@anachronic
Copy link
Copy Markdown
Owner

I don't know what's causing emacs 26.3 to fail, can we add it to allowed_failures in travis.yml?

@anachronic
Copy link
Copy Markdown
Owner

I'm not really sure this is an issue with chmod. I think it has to do with this line, which prevents the buffer of being edited when there's a merge conflict.

We could just remove that line altogether (with the added risk of having people modifying the lockfile, which isn't that terrible anyway) or set up a hook for ediff mode.

@mplscorwin
Copy link
Copy Markdown
Collaborator Author

mplscorwin commented Jan 4, 2020 via email

@mplscorwin
Copy link
Copy Markdown
Collaborator Author

mplscorwin commented Jan 4, 2020 via email

not sure 26.3 is failing, will research presently
@mplscorwin
Copy link
Copy Markdown
Collaborator Author

mplscorwin commented Jan 4, 2020 via email

@anachronic
Copy link
Copy Markdown
Owner

I wondered about that. If the issue is just that the buffer pops as read-only then maybe we should just document the need to press C-x q prior to making edits. Given the file is R/O I agree with the original design of popping it read-only. Add a custom to disable implicitly setting it read-only?

On Sat, Jan 4, 2020 at 9:18 AM Nicolás Salas V. @.***> wrote: I'm not really sure this is an issue with chmod. I think it has to do with this line

(setq buffer-read-only t))
, which prevents the buffer of being edited when there's a merge conflict. We could just remove that line altogether (with the added risk of having people modifying the lockfile, which isn't that terrible anyway) or set up a hook for ediff mode. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#5?email_source=notifications&email_token=AA6BAZWR4LWVOEHK33BP5N3Q4CSC7A5CNFSM4KBTLBXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEICZ4KI#issuecomment-570793513>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6BAZRL5YIPWEAVUFSXMMDQ4CSC7ANCNFSM4KBTLBXA .
-- Corwin 612-217-1742 612-298-0615 (fax) 612-695-4276 (mobile) corwin.brust (skype)corwin@bru.st corwin@bru.st

I don't know. It's been a while since I've used Emacs, and way more than that since I used ediff. I don't remember if one would need to C-x q all buffers or just one. If I'm lucky, I could try tonight or tomorrow night.

@mplscorwin
Copy link
Copy Markdown
Collaborator Author

mplscorwin commented Jan 4, 2020 via email

@anachronic
Copy link
Copy Markdown
Owner

anachronic commented Jan 5, 2020

I had the binding wrong -- I was thinking of C-x C-q which toggles read-only-mode. That is buffer local, of course, is similar to if we did our own save command and wrapped it in (let ((inhibit-read-only t)) ...save the file...)
-- Corwin 612-217-1742 612-298-0615 (fax) 612-695-4276 (mobile) corwin.brust (skype)corwin@bru.st corwin@bru.st

Yeah, sorry, I didn't remember the binding either. Thing is that ediff opens up multiple buffers so that you can view the diff or merge conflicts (or w/e, really) side to side. I don't remember if there's a minor mode for ediff. If there is, we could hook to it and (setq buffer-read-only nil)

@anachronic
Copy link
Copy Markdown
Owner

anachronic commented Jan 5, 2020

I've prepared an example repo for this issue here. Merge feature1 into master and there'll be a merge conflict.

;;;###autoload
(add-hook 'ediff-prepare-buffer-hook
          (lambda ()
            (when (equal major-mode 'yarn-mode)
              (progn
                (setq buffer-read-only nil)))))

I tried the snippet above and it does seem to work.

Btw, if you need to try out ediff, you can just e from magit when there's a merge conflict.

Edit: sorry, I left a message call that I used for debugging 😅

@mplscorwin
Copy link
Copy Markdown
Collaborator Author

This is pretty clearly the way to go with this. Sorry to disappear the latter half of the weekend. I'll jump back into this after work this evening.

@mplscorwin
Copy link
Copy Markdown
Collaborator Author

This is still on my plate to confirm the above solution works and update/write tests. Probably not this weekend. If happen to try patching with this we'd welcome any additional reportage.

@anachronic
Copy link
Copy Markdown
Owner

Great! I'm happy to hear that you still have this in your backlog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants