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

Make dirty files unique per source file #13

Merged
merged 2 commits into from
Nov 12, 2020
Merged

Conversation

RSDuck
Copy link
Contributor

@RSDuck RSDuck commented Nov 5, 2020

Apparently nimsuggest assumes this to be true and not doing can result in this: pragmagic/vscode-nim#180

@saem
Copy link
Owner

saem commented Nov 6, 2020

I read through the linked ticket, I think this would create lots of dirty files and we wouldn't have a particularly great way of knowing if we can clean them up.

Not an entire solution, but perhaps using the nimsuggest process pid as the discriminant tracked as a separate field as part of NimSuggestProcessDescription would be the right way to go? Even without being perfect (I'm ok with reasonable effort) the dirty files can be deleted on shutdown or detected crash of the associated nimsuggest process.

Thoughts?

@RSDuck
Copy link
Contributor Author

RSDuck commented Nov 6, 2020

I agree, I'll group them into an folder per nimsuggest instance and add some clean up code so that the files don't pile up.

and clean everything up upon closing
@RSDuck
Copy link
Contributor Author

RSDuck commented Nov 11, 2020

that's done now.

On a side note, I try to be consistent with style, though I've already made an issue about code formatting and I wonder what's the best way to go about this. There are also some other things which nimpretty doesn't change like grouping multiple variables inside one var block, which I know not everybody does (but I adore this syntax 😄), shall I just create a PRs for this and a few other things?

@saem
Copy link
Owner

saem commented Nov 12, 2020

Looks good to me, thanks!

For the formatting, I think the only thing I care about is 80 char line width, as it lets me have many sources files open side by side on my wide screen -- but I've broken that rule plenty. This was definitely a "figure out Nim as I go" project and so I'm not wedded to the current formatting or much else for that matter, I'll happily accept a PR.

@saem saem merged commit 9b99f44 into saem:master Nov 12, 2020
@saem
Copy link
Owner

saem commented Nov 12, 2020

FYI, I'm working on a release at the moment, new version should be out soon.

@JorySchossau
Copy link

@RSDuck Yay! Thanks. Our dev team thanks you, too.

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.

3 participants