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

Fixing Huffman Encoding edge case failure for assembly x64 #662

Merged
merged 2 commits into from
Dec 8, 2021

Conversation

Gathros
Copy link
Contributor

@Gathros Gathros commented Dec 23, 2019

No description provided.

@Gathros Gathros added the Implementation Edit This provides an edit to an algorithm implementation. (Code and maybe md files are edited.) label Dec 23, 2019
@leios
Copy link
Member

leios commented May 17, 2020

Is anyone actually able to review this? If not, I am happy to run it locally and try to figure it out, but I would need a little help figuring out how to do that.

@ntindle
Copy link
Member

ntindle commented Aug 28, 2021

[lang: asm]

@github-actions github-actions bot added the lang: asm Assembly programming language label Aug 28, 2021
Copy link
Contributor

@stormofice stormofice left a comment

Choose a reason for hiding this comment

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

The code looks good to me and it produces the correct output. Thanks!

[If anyone wants to test this themselves, change the text variable to something like aaaaaaaaaa and compile it with gcc -no-pie huffman.s]

@leios
Copy link
Member

leios commented Oct 6, 2021

Great! I am happy to merge this PR then! I will say that the comments to the code are slightly too long and do not render correctly in the AAA:
Screenshot from 2021-10-06 13-51-56

@stormofice
Copy link
Contributor

I think that [code] line length should be checked by a CI task / GitHub action or something similar in the future, so I think we can merge this one (and fix the line length later on with other ones that will fail when this gets enforced).

@ntindle
Copy link
Member

ntindle commented Dec 1, 2021

Imo we should aim for the editor to support wordwrap rather than have the maintainers be responsible for checking line length

@stormofice
Copy link
Contributor

I created an issue (see #962) to separate the discussion about long lines and unblock this PR.

@Amaras
Copy link
Member

Amaras commented Dec 8, 2021

Since this has been approved and a specific issue has been opened, I'm merging this in

@Amaras Amaras merged commit d127a81 into algorithm-archivists:main Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implementation Edit This provides an edit to an algorithm implementation. (Code and maybe md files are edited.) lang: asm Assembly programming language
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants