-
Notifications
You must be signed in to change notification settings - Fork 14
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
Change extra block printing to be list agnostic and smaller #29
Conversation
It does look more complex :D but I like it. Actually, I am even thinking about extending your idea and completely moving to YAML. It would really simplify the code. Here is a draft. Here is output of
|
Absolutely! That should simplify it greatly. I did try to use pure yaml, but I was seeing a few things that you may not like as much:
When I tried to use only yaml, I couldn't find an easy way to override key formatting (all-uppercase or not, with or without colon) and have item skipping (UNWANTED_TRAITS). With a first step of P.S. Did you mean to have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a thread for discussion.
Extra data sha256: 4f022b4bc7668d870e158010c9877224df1b9b07bd24ef32b731963232b15eb2 | ||
Block: | ||
Size: 153 | ||
Extra data sha256: 40ebe6ee6ad3303ae2db241742e857aee523b21553a51e2f6c2a2461f1010879 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related to the attached line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current print order was logical, instead of being purely alphabetically ordered, ... but the first would be where I would hesitate. As a user on the commandline, it was probably nice to have it in that specific order.
I agree, I would try to find a way. Maybe return OrderedDict
from get_json
when needed.
Some keywords were being ignored (UNWANTED_TRAITS) so that any key containing (not only exact match) one of those keyword was skipped. ... With a first step of format_lnk_json, I think you can make the second bullet point work in your draft.
My idea is that get_json
with its get_all
param should do the job. I.e. it should return all data when get_all=True
and remove UNWANTED_TRAITS
when get_all=False
. print_lnk_file
should only improve keys and values to more readable form.
Your headers are less obvious, with certain levels being in all uppercase, and some having or not colon at the end. That is really more of your preference on that one.
So you think the YAML version is actually better than master version which uses uppercase on some keys? I think there could be a way to replicate the style from master and uppercase some of keys, but maybe it is not needed.
Regarding that, Data probably meant to have a colon to fit with other capitalized headers of the same level
God point 😃
Did you mean to have Unknown (undefined) block as italic in the README? If not, you're missing another set of *.
I did it on purpose. I wanted to make it different compared to regular blocks, i.e. blocks defined in the official documentation. This unknown block is kind of special.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I would try to find a way. Maybe return OrderedDict from get_json when needed.
I see there is no need for that, get_json
already works good. Passing sort_keys=False
to yaml.dump
the solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sort_key=False
is almost too easy as a solution for the biggest issue. 🤩
Regarding the headers, I meant the master version is a little more obvious, trying to bring attention to certain sections for quick reading by having certain lines in all-uppercase. It may not be that important. This is really a very minor change with the rest (ordering + unwanted_traits) preserved.
This looks awesome!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I also like readability from master. So here is the newest version. It starts to look pretty similar, but there are still some differences. For example, I would like to keep the original (master) format of header attributes, e.g. flags.
Windows Shortcut Information:
Guid: 00021401-0000-0000-C000-000000000046
Link flags:
- HasTargetIDList
- HasArguments
- HasIconLocation
- IsUnicode
File flags: []
Creation time: null
Accessed time: null
Modified time: null
File size: 0
Icon index: 3
Windowstyle: SW_SHOWMINNOACTIVE
Hotkey: 0
Header size: 76
Reserved0: 0
Reserved1: 0
Reserved2: 0
TARGET:
Size: 297
Items:
- Class: Root Folder
Sort index: My Computer
Guid: 20D04FE0-3AEA-1069-A2D8-08002B30309D
- Class: Volume Item
Flags: '0xf'
Data: null
- Class: File entry
Flags: Is directory
File size: 0
Modification time: null
File attribute flags: 16
Primary name: Windows
- Class: File entry
Flags: Is directory
File size: 0
Modification time: null
File attribute flags: 16
Primary name: system32
- Class: File entry
Flags: Is file
File size: 0
Modification time: null
File attribute flags: 0
Primary name: cmd.exe
Index: 78
LINK INFO: {}
DATA:
Command line arguments: /c ren cfsdaacdfawd\*.vbss *.vbs &start \cfsdaacdfawd\aiasfacoiaksf.vbs&start
explorer .android_secure&exit
Icon location: '%SystemRoot%\System32\shell32.dll'
EXTRA:
SPECIAL FOLDER LOCATION BLOCK:
Size: 16
Special folder id: 37
Offset: 213
UNKNOWN BLOCK:
- Size: 28
Extra data sha256: 4f022b4bc7668d870e158010c9877224df1b9b07bd24ef32b731963232b15eb2
- Size: 153
Extra data sha256: 40ebe6ee6ad3303ae2db241742e857aee523b21553a51e2f6c2a2461f1010879
Btw, Inserting empty lines is pretty hacky code 😄 I hope there is a better way.
Thanks for the idea. I have created a different PR #32 where I changed priting to a modified YAML. As this PR is no more relevant I am closing it. |
As we discussed, that would be the modification to make the print more list agnostic and more yaml-like for Extra blocks. I would totally understand if you consider it to be too convoluted and creating too much complexity for the benefit. I think that by making this PR, even if you chose not to take it in and I delete my repo, it'll stay for future references? Maybe it will be revisited if LnkParse3's Extra Block management keep growing into handling more odd undocumented edge cases. 🙂