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

Add support for fortran legacy #519

Merged
merged 8 commits into from
Oct 27, 2021
Merged

Add support for fortran legacy #519

merged 8 commits into from
Oct 27, 2021

Conversation

jake-87
Copy link
Contributor

@jake-87 jake-87 commented Oct 26, 2021

Using F77 version of the F90 logo, as f77 is the most used "legacy" version of fortran

#490

@jake-87
Copy link
Contributor Author

jake-87 commented Oct 26, 2021

Apologies, o2sh, you may have to do the formatting, as due to an issue with the editor I am using (i think) I cannot actually get the rustfmt check to pass.

@spenserblack
Copy link
Collaborator

🤔 I don't think I've seen those errors before. Looks like it's trying to enforce tab indentation over space indentation. In which case your editor is actually behaving correctly AFAIK, according to EditorConfig.

indent_size = 4

AFAIK the preferred Rust format is to use 4 spaces for indentation, but perhaps a different indentation style is preferred in macro calls?

Are you able to update rustfmt and run it locally?

The issue you're having does bring up another issue though: ideally, it should be the diff that has its format checked, not the whole file (kind of like what lint-staged does for npm modules).

@jake-87
Copy link
Contributor Author

jake-87 commented Oct 26, 2021

I think i've found the problem: apon pasting part of the diff into [https://www.soscisurvey.de/tools/view-chars.php](a tool that shows non printable chars), it has become apparent that the rest of the macro is indeed formatted with tabs. I will attempt some copy pasting to fix.

@jake-87
Copy link
Contributor Author

jake-87 commented Oct 26, 2021

Of course, ideally I would run rustfmt locally to understand why it's showing this behavure of tabs over spaces, but unfortunately I am not able to do that right now. Might be able to try in a few hours when I have access to rustfmt.

@jake-87
Copy link
Contributor Author

jake-87 commented Oct 26, 2021

Ah, passes now. Copy paste saves the day.

@jake-87
Copy link
Contributor Author

jake-87 commented Oct 27, 2021

for anyone viewing this PR, i've made a discussion post to try and narrow down whats causing issues with rustfmt: o2sh/onefetch/discussions/521

@o2sh
Copy link
Owner

o2sh commented Oct 27, 2021

Thanks for your contribution @jake-87. LGTM 👍
Concerning the rust fmt issue, this was most likely introduced by f34303b. I don't mind using spaces over tabs, in which case, we should remove the line hard_tabs = true from the .rustfmt.toml to be more consistent with .editorconfig

@jake-87
Copy link
Contributor Author

jake-87 commented Oct 27, 2021

Sounds good to me.

jake-87 added a commit to jake-87/onefetch that referenced this pull request Oct 27, 2021
@jake-87 jake-87 mentioned this pull request Oct 27, 2021
@o2sh o2sh merged commit 4ad136a into o2sh:main Oct 27, 2021
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