Skip to content

Mabofoul/order in tokentypes#3

Closed
NoneSince wants to merge 3 commits into
Thomas-995:masterfrom
NoneSince:mabofoul/order_in_tokentypes
Closed

Mabofoul/order in tokentypes#3
NoneSince wants to merge 3 commits into
Thomas-995:masterfrom
NoneSince:mabofoul/order_in_tokentypes

Conversation

@NoneSince

Copy link
Copy Markdown

reordered some structs and functions to mach the order of enum CON_TOKENTYPE

NoneSince and others added 3 commits January 25, 2024 00:44
…s of header files, removed boost dependancy, removed/added spaces. included @xtrm-en work of PR #1, except his makefile (his makefile is better but i wont copy others work)

@Thomas-995 Thomas-995 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Like these changes a lot, especially the exception handling stuff. The only thing I'm not sure about is removing the boost library and throwing a bunch of string functions in the deconstruct file. If this could be removed or done differently I'll be merging this commit for sure.

@NoneSince

NoneSince commented Jan 31, 2024

Copy link
Copy Markdown
Author

@Thomas-de-Bock, first of all, this PR was sorted better in PR #4 (step by step), so i would like use to take there, not here. this PR I will close after we finish discussion.
Also, PR #5 builds on PR #4, and just adds one small thing, so you can go to it only after PR #4, and look only at the last 2 commits, not all the changes including PR #4.

Regarding this commit comment:
there is no need for non standard libraries, similar to the fact that there is no need for frameworks, other languages, or OS-specific files.
I did more than what the project needs, I supplied the developers (you specifically) with a function that acts almost like boost::split and takes same args. We don't actually nned that, since our use case is limited to a set/type of input, so no need for IsAnyOF class, no need for it to be a template, and no need for compress and strip flags.
No matter how ugly it looks, it is still better that a non-standard library, but fortunatilly it doesnt need to be ugly:

  1. I can put a declaration on to of the file, and move the implementation to the bottom, so all the extra fuctions can be not looked at.
  2. The more futuristic approach is to have a file for utility functions like this. we will definitly do this if there is another file in need of split function, but i would like to delay that because approach 1 is enough, and matches the way you did split your files.

Regarding commit #5 comment:
reg_to_str always received the global variable, so I removed the argument to show what it really is. That's the only purpose of the function, so why make it extra flexible? We, on purpose, want it to be limited t it's use case, to be obvious and not confused for more than what it is. Only if it becomes needed for something extra, only then you make it support extra cases.
split is static because it is a "quick calculation on the side", it is a helper function not needed for its own value. The meaning of static is not "won't be used elsewhere", it means "this function doesn't meat the purpose f the file". If we want to use split in another file, we don't include "destruct.h" and make it not static, we rather create a new file to hold it not statis, and include that file.
_con_condition, same thing like static, is a type only used within construct_types.h, to build con_while and con_if, to give these two types to other files. So, _con_condition is a helper type internal for the file, unlike the other types, so i showed that with the "_" I added to it's name.

@NoneSince

NoneSince commented Jan 31, 2024

Copy link
Copy Markdown
Author

I suggest you merge this PR and keep split function for another PR (create ISSUE for that, so whoever wants can do, not delaying any PR for that), because this PR is big and changes in many places, so it will be hard to keep up to date, and hard for other developers to restart again from it if you delay it.

This was referenced Jan 31, 2024
@NoneSince NoneSince closed this Feb 2, 2024
@NoneSince NoneSince deleted the mabofoul/order_in_tokentypes branch February 2, 2024 08:19
@NoneSince NoneSince restored the mabofoul/order_in_tokentypes branch February 2, 2024 08:19
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