Skip to content

Allow files with CR newline #43

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

molsonkiko
Copy link

@molsonkiko molsonkiko commented Mar 13, 2023

Previously files with CRLF and LF newlines were supported, but not CR. Address #41 .

Added a test for this.

Also fixed an annoying UI problem with the query form where hitting enter or tab would cause a dinging noise.

I ran tests on my computer and the only tests that failed were the MSSQL tests (which is expected because SQL Server is not installed on my computer).

@molsonkiko
Copy link
Author

I tried running CsvQuery on NPP 7.3.3 64bit and this plugin made Notepad++ crash. This doesn't happen on NPP 7.3.3 32bit.
That said, your last release also had this issue, so I'm not going to close my PR.

@jokedst
Copy link
Owner

jokedst commented Mar 14, 2023

NPP did some changes to the x64 internal workings in 8.3, so a plugin released after that (1.2.9) will not work with a N++ version before 8.3. If you HAVE to use a v7 of N++, you have to use an old version of CsvQuery as well, i.e. v1.2.8 or older.

@molsonkiko
Copy link
Author

Thanks for the tip about older versions. I just keep a lot of NPP versions installed on my computer for testing.

If you wanted the minimum viable fix for this bug, I would just apply my simple changes to CsvSettings.cs and keep my new test and my new test file.

@jokedst
Copy link
Owner

jokedst commented Mar 15, 2023 via email

jokedst pushed a commit that referenced this pull request Nov 9, 2024
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