jeremiah-c-leary / vhdl-style-guide

Style guide enforcement for VHDL
GNU General Public License v3.0
182 stars 38 forks source link

Add linesep configuration option #1012

Closed alonbl closed 10 months ago

alonbl commented 11 months ago

Description

Platform independent line ends are important to avoid conflicts.

Additional context

Resolves: #1010

alonbl commented 11 months ago

Very difficult to work without enforcing eol, the windows people always finds git complaining after they run the reformat.

codecov[bot] commented 11 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (a35ff96) 95.87% compared to head (a12bfb2) 95.90%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1012 +/- ## ========================================== + Coverage 95.87% 95.90% +0.03% ========================================== Files 1500 1500 Lines 28273 28269 -4 ========================================== + Hits 27106 27111 +5 + Misses 1167 1158 -9 ``` | [Files](https://app.codecov.io/gh/jeremiah-c-leary/vhdl-style-guide/pull/1012?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jeremiah+Leary) | Coverage Δ | | |---|---|---| | [vsg/apply\_rules.py](https://app.codecov.io/gh/jeremiah-c-leary/vhdl-style-guide/pull/1012?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jeremiah+Leary#diff-dnNnL2FwcGx5X3J1bGVzLnB5) | `89.18% <100.00%> (ø)` | | | [vsg/vhdlFile/utils.py](https://app.codecov.io/gh/jeremiah-c-leary/vhdl-style-guide/pull/1012?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jeremiah+Leary#diff-dnNnL3ZoZGxGaWxlL3V0aWxzLnB5) | `89.18% <87.50%> (+1.24%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

alonbl commented 11 months ago

I added ANSI test which was missing in master. I do not know if/how you want stdin test.

alonbl commented 11 months ago

Hi @jeremiah-c-leary,

Can you please review this patch? It is important patch for mixed POSIX/Windows collaboration. I added stdin and ANSI coverage tests, I am almost sure the coverage tool is wrong.

Thanks, Alon

jeremiah-c-leary commented 10 months ago

Morning @alonbl ,

I tried this out on your alonbl branch, but it did not seem to work as I was expecting. In order to honor the \r\n option I had to add \r into line 157:

152 def write_vhdl_file(oVhdlFile, dConfig):
153     tmpfile = f"{oVhdlFile.filename}.tmp"
154     try:
155         with open(tmpfile, 'w', encoding='utf-8', newline=dConfig.get("linesep")) as oFile:
156             for sLine in oVhdlFile.get_lines()[1:]:
157                 oFile.write(sLine + '\r\n')
158         os.replace(tmpfile, oVhdlFile.filename)
159     except PermissionError as err:
160         print (err, "Could not write fixes back to file.")
161     finally:
162         try:
163             os.remove(tmpfile)
164         except FileNotFoundError:
165             pass

When I added the \r Vim detected the output file as dos.

Could you confirm on your end?

Thanks,

--Jeremy

alonbl commented 10 months ago

Hi @jeremiah-c-leary ,

I need more information. What platform did you use? Have you forced specific encoding? As far as I tested python on windows will translate \n to \r\n automatically when opened as text, in *NIX it will keep \n.

Thanks,

jeremiah-c-leary commented 10 months ago

Hey @alonbl ,

Is it possible to write out a file on Windows with \n as the line ending?

--Jeremy

alonbl commented 10 months ago

Is it possible to write out a file on Windows with \n as the line ending?

Do you mean to enforce that? this what this patch is doing?

Or do you mean to use these files? Sure, and we also enforce this in git using the following .gitattributes:

[core]
* text=auto eol=lf

All the tools, editors supports LF style files, some of them open new files with CRLF, but once git fixes that everything is consistent. The repositories never accepts CRLF styles.

jeremiah-c-leary commented 10 months ago

Morning @alonbl ,

I was trying to figure out why I could not generate dos line endings while on linux. The documentation for open seems to indicate it is possible:

image

...but then why doesn't setting linesep to \r\n:

linesep:  "\r\n"

cause line 157

152 def write_vhdl_file(oVhdlFile, dConfig):
153     tmpfile = f"{oVhdlFile.filename}.tmp"
154     try:
155         with open(tmpfile, 'w', encoding='utf-8', newline=dConfig.get("linesep")) as oFile:
156             for sLine in oVhdlFile.get_lines()[1:]:
157                 oFile.write(sLine + '\n')
158         os.replace(tmpfile, oVhdlFile.filename)
159     except PermissionError as err:
160         print (err, "Could not write fixes back to file.")
161     finally:
162         try:
163             os.remove(tmpfile)
164         except FileNotFoundError:
165             pass

write out line endings of \r\n?

I have not tested the windows side, but I think the following is occuring:

platform linesep line ending Comment
*nix None \n
*nix \n \n because line 157 appends \n
*nix \r\n \n I would assume it would be \r\n
Windows None \r\n
Windows \n \n because line 157 appends \n
Windows \r\n \n I would assume it would be \r\n

So I am confused why the \n in line 157 doesn't appear to be updated when using \r\n.

I am going to try some more experiments.

--Jeremy

jeremiah-c-leary commented 10 months ago

Morning @alonbl ,

Nevermind. I must have been doing something wrong. My experiment with '\r\n` is working now.

--Jeremy

alonbl commented 10 months ago

Closed via #1017