Skip to content

Fix cwd issues when using Neomake #261

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

Closed
wants to merge 1 commit into from

Conversation

dpc
Copy link

@dpc dpc commented Sep 9, 2018

All credit to @blueyed .

This fixes #260

The investigation is in the corresponding Neomake issue:
neomake/neomake#2080

This is also related to: #259

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @chris-morgan (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@dpc dpc changed the title Fix a cwd issues when using Neomake Fix cwd issues when using Neomake Sep 9, 2018
call writefile(l:content, expand('%'))
silent edit!
normal! ggdG
call setline(1, l:content)
let &syntax = &syntax
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The let &syntax = &syntax is also not necessary anymore likely (if it ever was, but maybe due to the edit that was used previously).

TLDR of the investigation: the problem is that expand('%') here might be relative, but it was :lcded before.
An alternative might be to use the full buffer name (save it before cding), but I think just setting the lines is better than writing from outside.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually expand('%') should be the relative path after :lcd - so there is some other reason for this (maybe a bug in Vim/Neovim even (NVIM v0.3.1 used by the reporter)).

Anyway, the cding back should be done in a try … finally block, because otherwise it might not change it back.

@blueyed
Copy link

blueyed commented Sep 9, 2018

btw: as can be seen in neomake/neomake#2080 (comment) the error/issue shows up immediately when not using :silent! - therefore this should really be avoided, since it hides all kind of errors.

@@ -146,8 +146,8 @@ function! s:RunRustfmt(command, tmpname, fail_silently)
let l:content = readfile(a:tmpname)
endif

call writefile(l:content, expand('%'))
silent edit!
normal! ggdG
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed that this should become normal! "_ggdG to not mess with registers.

@blueyed
Copy link

blueyed commented Sep 9, 2018

Please remove the mentioning of me before merging it, otherwise it will show up in notifications all over the place. Use @blueyed maybe in backticks or so.

@dpc dpc force-pushed the neomake-workspaces branch from ef7bb15 to 2cecf06 Compare September 9, 2018 07:33
All credit to `@blueyed`.

This fixes rust-lang#260

The investigation is in the corresponding Neomake issue:
neomake/neomake#2080

This is also related to: rust-lang#259
@dpc dpc force-pushed the neomake-workspaces branch from 2cecf06 to 8a17725 Compare September 9, 2018 07:54
@blueyed
Copy link

blueyed commented Sep 9, 2018

Thanks for trying - I was still notified of the new commits though.. :/

@da-x
Copy link
Member

da-x commented Sep 9, 2018

Hi @blueyed and @dpc ,

Thanks for debugging the issue.

However the problem with the normal! "_ggdG and call setline(1, l:content) sequence is that it's a regression over issue #236. We should also prevent undo actions from being affected by this fix.

@blueyed
Copy link

blueyed commented Sep 9, 2018

@da-x
I see (2fb3df5).
I've looked at neoformat, and they use a similar approach: https://github.com/sbdchd/neoformat/blob/master/autoload/neoformat.vim#L170
Maybe it is more related to the (missing/wrong) undojoin, and or mkview! etc?

@dpc
Copy link
Author

dpc commented Sep 9, 2018

@blueyed I've seen you landed something in neomake itself? Does it mean we should close this one?

@blueyed
Copy link

blueyed commented Sep 9, 2018

@dpc
Not sure. I think it still is a bit fragile what is being done in rust.vim, but there appear to be reasons, and I am not using it myself anyway.

I suggest closing this for now - it can get re-opened later.

@da-x
Copy link
Member

da-x commented Sep 9, 2018

@dpc @blueyed Looking over neoformat, I pushed 4c21791 to master. It should do something similar this PR, but without regressing over #236. The existing undojoin seems to work with this change without any cursor funkiness on u. I wished though that winsaveview and winrestview would work right like in neoformat but was not able to achieve that. Feel free to test.

@blueyed
Copy link

blueyed commented Sep 9, 2018

@da-x
Looks good, thanks!

@dpc
Copy link
Author

dpc commented Sep 9, 2018

I've updated all vim plugins to latest and will keep working on my workspace-enabled project. I'll report any issues. Thanks everyone for looking into this!

@dpc dpc closed this Sep 9, 2018
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.

Automatic rustfmt erases my files sometimes.
5 participants