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

bugfix for Incorrect encoding #548

Merged
merged 1 commit into from
Jun 15, 2024
Merged

bugfix for Incorrect encoding #548

merged 1 commit into from
Jun 15, 2024

Conversation

fengzyf
Copy link
Contributor

@fengzyf fengzyf commented Jun 9, 2024

what is this pull request about?

fix for Incorrect encoding (possiblely due to #505)

does this relate to an existing issue?

#547

does this change any existing behaviour?

yes

what does this add?

Use Windows:: GetACP() to obtain the code page of the Windows system, and then obtain the system default encoding. Users do not need to specify the encoding.

@qiancy98
Copy link
Contributor

qiancy98 commented Jun 9, 2024

感谢,看起来很优雅(至少比我原来写的优雅hhh)

@cmhughes
Copy link
Owner

cmhughes commented Jun 9, 2024 via email

@qiancy98
Copy link
Contributor

qiancy98 commented Jun 9, 2024

I'm sorry, I don't speak this language

On Sun, 9 Jun 2024, 11:41 qiancy98, @.> wrote: 感谢,看起来很优雅(至少比我原来写的优雅hhh) — Reply to this email directly, view it on GitHub <#548 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ7CYA7BP5P2MJ73SQFNGDZGQWO7AVCNFSM6AAAAABJAW3JFCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJWGQZTOMJYGM . You are receiving this because you are subscribed to this thread.Message ID: @.>

That reads

Thanks for your coding. This looks elegent. (at least more elegent than what I typed.)

This is just a thanks.

@cmhughes
Copy link
Owner

cmhughes commented Jun 9, 2024 via email

sub copy_with_encode {
use File::Copy;
my ( $source, $destination ) = @_;

if ( $FindBin::Script eq 'latexindent.exe' ) {
if ( $FindBin::Script =~ /\.exe$/ ) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why does this line need changing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why does this line need changing?

Users may change the name of the Windows executable file, such as changing latexindent.exe to latexindent3.24.1.exe, and the code should still take effect.

@cmhughes
Copy link
Owner

Thanks for you help with this.

Does this mean that https://latexindentpl.readthedocs.io/en/latest/sec-appendices.html#encoding-indentconfig-yaml should be removed? and all references to encoding should be removed?

@fengzyf
Copy link
Contributor Author

fengzyf commented Jun 11, 2024

Does this mean that https://latexindentpl.readthedocs.io/en/latest/sec-appendices.html#encoding-indentconfig-yaml should be removed? and all references to encoding should be removed?

Certainly, they need to be removed.

Perhaps new explanations about encoding can be added to inform users:

  1. For the Windows executable file latexindent.exe, its options support UTF-8 characters.
  2. For the Windows Perl script latexindent.pl, its options support characters encoded according to the system code page. You can check the system code page by using chcp in the command prompt, and refer to the supported characters at Microsoft's code page identifier table.
  3. For Ubuntu Linux and macOS users, whether using the Perl script or the executable file, the options support UTF-8 characters.
  4. When reading and writing files, the files are read and written in UTF-8 format by default. That is to say, the encoding format for tex and yaml files needs to be in UTF-8 format.

@cmhughes
Copy link
Owner

cmhughes commented Jun 11, 2024 via email

@cmhughes
Copy link
Owner

The file is documentation/sec-appendices.tex

@fengzyf
Copy link
Contributor Author

fengzyf commented Jun 11, 2024

I have updated the document, but I am not very proficient in English. Therefore, I hope you can help me check it and make any necessary modifications.

@cmhughes
Copy link
Owner

Something isn't right with this... can you look at the output at https://github.com/cmhughes/latexindent.pl/actions/runs/9468805527/job/26086016558?pr=548 ?

@cmhughes
Copy link
Owner

Thanks.

This still isn't working https://github.com/cmhughes/latexindent.pl/actions/runs/9488246197/job/26149866332?pr=548

(I appreciate I need to make it more obvious that the checks fail)

@fengzyf
Copy link
Contributor Author

fengzyf commented Jun 13, 2024

https://github.com/cmhughes/latexindent.pl/actions/runs/9502252857/job/26189794904

This new commit still triggers an error:
Error: Attempt to call undefined import method with arguments ("$ifElseFiBasicRegExp") via package "LatexIndent::IfElseFi" (Perhaps you forgot to load the package?) at /usr/local/bin/LatexIndent/Special.pm line 25.

According to my Google search results, it may have appeared after Perl was updated to 5.39.9.
see https://perldoc.perl.org/5.39.9/perl5391delta.

New Errors
Attempt to call undefined %s method with arguments via package "%s" (perhaps you forgot to load the package?)
(F) You called the import() or unimport() method of a class that has no import method defined in its inheritance graph, and passed an argument to the method. This is very often the sign of a misspelled package name in a use or require statement that has silently succeeded due to a case-insensitive file system.
Another common reason this may happen is when mistakenly attempting to import or unimport a symbol from a class definition or package which does not use Exporter or otherwise define its own import or unimport method.

I don't know how to fix it. However, if you change the Perl version from 'latest' to 5.38 in the 'test-cases-on-pull.yaml', this error message will not appear.

@cmhughes
Copy link
Owner

Many thanks, that's helpful.

I've fixed this as of 339d396

Can you re-base your branch and push another commit?

@cmhughes cmhughes merged commit c357c9a into cmhughes:develop Jun 15, 2024
2 checks passed
@cmhughes
Copy link
Owner

Thanks so much for your work on this! I really appreciate it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants