-
Notifications
You must be signed in to change notification settings - Fork 44
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
add features from other Dasm forks like improved incbin #92
Comments
While the whole point of open source is to allow people to fork the code and add their own features, I think the goal in the long run is to keep a single point of contact for distribution of dasm. I think the task here will be to identify the changes made in this version (of which I was totally unaware until now) and integrate anything that makes any sense into the current dasm. My first thoughts, since it was branched 2.20.11, is to grab the source for both and do a diff. I may tackle that today. |
My very first comment on this new code is the addition of this notice...
IMHO you just can't do that. You can't back-copyright stuff that isn't your creation, and is in the public domain. |
On review, the codebase is changed too much to warrant spending time on trying to trace the individual issues. In particular, a large number of changes have been made to adjust the code to the new author's personal style.
There's more. Just way too many changes for me to even consider hunting for the gold. The author has clearly put a lot of time into making this version, but I don't think what I see in the source is of much use to us at this stage. I'm not saying all the changes made are wrong - but many are related to personal preference on style, not correctness or functionality. The new features seem to be obscured amongst all the other changes. My diff/comparison tool is a sea of red (changed) lines. I was unable, given a short time, to identify what the new features are. I think if we want to merge the code, we need to approach the author and ask some questions. Also, perhaps that new copyright notice should be challenged! Copyright (c) 2019 would be fine. Not encompassing the 10 year period prior to that! |
I'm not entirely sure who made the style changes, now. The SVN version 2.10.11 (which is numbered 2.20.12 internally) appears to be pre-PFH, so much of the mass of red may be because I'm comparing an incorrect version....? |
Wouldn't it be ironic if I was the origin of those changes... LOL |
Merging stuff might be too much work. Maybe integrating some ideas or additions like the incbin stuff might be easier. I switched from the old 2.10.11 an 2.10.12 to iAN CooGs version, because they had serious problems correctly assembling stuff using rorg rend. |
I could not find ANY documentation in the likely places that list the additions/changes. |
Here are the changes from iAN CooGs Dasm which was special tailored for the c64 (from the dasm.doc: VERSIONS: v2.20.07b
v2.20.07c
label Now can be written v2.20.07d
v2.20.07e
v2.20.07f
v2.20.07g
Interface:
New / improved commands:
v2.20.07h
v2.20.07i
xxxxxxxxxxxxxxxxxxxxx[etc over 256chars]xxx: the ":" probably means it was expected to be interpreted as a label (OMG!) v2.20.07j
v2.20.07k
v2.20.07-iAN-rev-L 20080508
v2.20.07-iAN-rev-M 20141107
v2.20.07-iAN-rev-N 20160125
main subroutine this produced a symbol list like this: al C:$1000 .main the extra "1." is added by DASM internally to distinguish local labels that can al C:$1000 .main |
Here the ones from TLR: README.tlrThis document describes a set of patches for dasm written by tlr. 1: Forced operands now really force. Previously a .z wouldn't hard set before: It would also lead to problems with code in zero page.
i1:
|
TY. Great info and useful for discussion of what to import/include for a merge. |
I got info from tlr which might be useful: In my first 2009 release ( dasm-2.20.11-r323-20090221-tlr ), the r323 version that everything was originally forked from is included unmodified. That may be useful for Andrew to help in extracting relevant patches. |
I'm missing some nice additions from other Dasm forks like the ones from tlr or iAN CooG:
https://csdb.dk/release/?id=182815
http://iancoog.altervista.org/
For example:
incbin [,[,]]
FNAME eqm "file4.bin"
a:
incbin "file1.bin"
b:
incbin "file2.prg",2
incbin "file3.bin",0,16
incbin FNAME, 0, b-a
And a lot more was added and fixed.
The text was updated successfully, but these errors were encountered: