From b15c4cd15a518a4e22e44dcb029078b549b16d5c Mon Sep 17 00:00:00 2001
From: Mahmoud-Emad <mahmmoud.hassanein@gmail.com>
Date: Tue, 5 Aug 2025 19:02:26 +0300
Subject: [PATCH] refactor: Change hero action syntax to verb.noun

- Change action name format from `obj.verb` to `verb.obj`
- Update decoder to look for `define.obj` or `configure.obj`
- Modify encoder export to use the new `define.obj` prefix
- Update all test constants and scripts to the new syntax
- Make Remark struct public for test visibility
---
 lib/data/encoderhero/decoder.v                |   6 +-
 lib/data/encoderhero/decoder_test.v           |  26 +--
 lib/data/encoderhero/encoder.v                |   2 +-
 .../encoderhero/encoder_ignorepropery_test.v  |   2 +-
 lib/data/encoderhero/encoder_test.v           |  12 +-
 .../postgres_client_decoder_test.v            | 134 +++++++------
 lib/data/paramsparser/params_reflection.v     | 181 ++++++++++--------
 test_basic.vsh                                |   2 -
 8 files changed, 187 insertions(+), 178 deletions(-)

diff --git a/lib/data/encoderhero/decoder.v b/lib/data/encoderhero/decoder.v
index 57a92ca3..4f656071 100644
--- a/lib/data/encoderhero/decoder.v
+++ b/lib/data/encoderhero/decoder.v
@@ -20,11 +20,11 @@ fn decode_struct[T](_ T, data string) !T {
 	// println(data)
 	$if T is $struct {
 		obj_name := texttools.snake_case(T.name.all_after_last('.'))
-		mut action_name := '${obj_name}.define'
+		mut action_name := 'define.${obj_name}'
 		if !data.contains(action_name) {
-			action_name = '${obj_name}.configure'
+			action_name = 'configure.${obj_name}'
 			if !data.contains(action_name) {
-				return error('Data does not contain action name: ${obj_name}.define or ${action_name}')
+				return error('Data does not contain action name: define.${obj_name} or ${action_name}')
 			}
 		}
 		actions_split := data.split('!!')
diff --git a/lib/data/encoderhero/decoder_test.v b/lib/data/encoderhero/decoder_test.v
index 355fd770..f149023b 100644
--- a/lib/data/encoderhero/decoder_test.v
+++ b/lib/data/encoderhero/decoder_test.v
@@ -40,10 +40,14 @@ struct ComplexStruct {
 	child ChildStruct
 }
 
-const blank_complex = '!!define.complex_struct'
-const partial_complex = '!!define.complex_struct id: 42 name: testcomplex'
+const blank_complex = '!!define.complex_struct
+!!define.child_struct'
+
+const partial_complex = '!!define.complex_struct id: 42 name: testcomplex
+!!define.child_struct'
+
 const full_complex = '!!define.complex_struct id: 42 name: testobject
-!!define.complex_struct.child text: child_text number: 24
+!!define.child_struct text: child_text number: 24
 '
 
 fn test_decode_complex() ! {
@@ -128,14 +132,14 @@ const person = Person{
 }
 
 fn test_decode() ! {
-	mut object := decode[Person]('')!
-	assert object == Person{}
-
-	object = decode[Person](person_heroscript)!
+	// Test decoding with proper person data
+	object := decode[Person](person_heroscript)!
 	assert object == person
 
-	// object = decode[ComplexStruct](full_complex) or {
-	// 	assert true
-	// 	ComplexStruct{}
-	// }
+	// Test that empty string fails as expected
+	decode[Person]('') or {
+		assert true // This should fail, which is correct
+		return
+	}
+	assert false // Should not reach here
 }
diff --git a/lib/data/encoderhero/encoder.v b/lib/data/encoderhero/encoder.v
index 1a999cc4..c973eb96 100644
--- a/lib/data/encoderhero/encoder.v
+++ b/lib/data/encoderhero/encoder.v
@@ -39,7 +39,7 @@ pub fn encode[T](val T) !string {
 // export exports an encoder into encoded heroscript
 pub fn (e Encoder) export() !string {
 	mut script := e.params.export(
-		pre:        '!!${e.action_names.join('.')}.configure'
+		pre:        '!!define.${e.action_names.join('.')}'
 		indent:     '	'
 		skip_empty: true
 	)
diff --git a/lib/data/encoderhero/encoder_ignorepropery_test.v b/lib/data/encoderhero/encoder_ignorepropery_test.v
index ea8f4fb0..bdac9eaa 100644
--- a/lib/data/encoderhero/encoder_ignorepropery_test.v
+++ b/lib/data/encoderhero/encoder_ignorepropery_test.v
@@ -12,7 +12,7 @@ struct MyStruct {
 }
 
 // is the one we should skip
-struct Remark {
+pub struct Remark {
 	id int
 }
 
diff --git a/lib/data/encoderhero/encoder_test.v b/lib/data/encoderhero/encoder_test.v
index a4ce306a..f0480209 100644
--- a/lib/data/encoderhero/encoder_test.v
+++ b/lib/data/encoderhero/encoder_test.v
@@ -10,7 +10,7 @@ struct Base {
 	remarks []Remark
 }
 
-struct Remark {
+pub struct Remark {
 	text string
 }
 
@@ -75,8 +75,7 @@ struct Profile {
 	url      string
 }
 
-const person_heroscript = "
-!!define.person id:1 name:Bob birthday:'2012-12-12 00:00:00'
+const person_heroscript = "!!define.person id:1 name:Bob age:21 birthday:'2012-12-12 00:00:00'
 !!define.person.car name:'Bob\\'s car' year:2014
 !!define.person.car.insurance provider:insurer
 
@@ -107,15 +106,14 @@ const person = Person{
 	]
 }
 
-const company_script = "
-!!define.company name:'Tech Corp' founded:'2022-12-05 20:14'
-!!define.company.person id:1 name:Bob birthday:'2012-12-12 00:00:00'
+const company_script = "!!define.company name:'Tech Corp' founded:'2022-12-05 20:14'
+!!define.company.person id:1 name:Bob age:21 birthday:'2012-12-12 00:00:00'
 !!define.company.person.car name:'Bob\\'s car' year:2014
 !!define.company.person.car.insurance provider:insurer
 
 !!define.company.person.profile platform:Github url:github.com/example
 
-!!define.company.person id:2 name:Alice birthday:'1990-06-20 00:00:00'
+!!define.company.person id:2 name:Alice age:30 birthday:'1990-06-20 00:00:00'
 !!define.company.person.car name:'Alice\\'s car' year:2018
 !!define.company.person.car.insurance 
 
diff --git a/lib/data/encoderhero/postgres_client_decoder_test.v b/lib/data/encoderhero/postgres_client_decoder_test.v
index b38b8ecb..fef6f715 100644
--- a/lib/data/encoderhero/postgres_client_decoder_test.v
+++ b/lib/data/encoderhero/postgres_client_decoder_test.v
@@ -1,6 +1,5 @@
 module encoderhero
 
-
 pub struct PostgresqlClient {
 pub mut:
 	name     string = 'default'
@@ -11,14 +10,13 @@ pub mut:
 	dbname   string = 'postgres'
 }
 
+const postgres_client_blank = '!!define.postgresql_client'
+const postgres_client_full = '!!define.postgresql_client name:production user:app_user port:5433 host:db.example.com password:secret123 dbname:myapp'
+const postgres_client_partial = '!!define.postgresql_client name:dev host:localhost password:devpass'
 
-const postgres_client_blank = '!!postgresql_client.configure'
-const postgres_client_full = '!!postgresql_client.configure name:production user:app_user port:5433 host:db.example.com password:secret123 dbname:myapp'
-const postgres_client_partial = '!!postgresql_client.configure name:dev host:localhost password:devpass'
-
-const postgres_client_complex = "
-!!postgresql_client.configure name:staging user:stage_user port:5434 host:staging.db.com password:stagepass dbname:stagingdb
-"
+const postgres_client_complex = '
+!!define.postgresql_client name:staging user:stage_user port:5434 host:staging.db.com password:stagepass dbname:stagingdb
+'
 
 fn test_postgres_client_decode_blank() ! {
 	mut client := decode[PostgresqlClient](postgres_client_blank)!
@@ -63,14 +61,14 @@ fn test_postgres_client_decode_complex() ! {
 fn test_postgres_client_encode_decode_roundtrip() ! {
 	// Test encoding and decoding roundtrip
 	original := PostgresqlClient{
-		name: 'testdb'
-		user: 'testuser'
-		port: 5435
-		host: 'test.host.com'
+		name:     'testdb'
+		user:     'testuser'
+		port:     5435
+		host:     'test.host.com'
 		password: 'testpass123'
-		dbname: 'testdb'
+		dbname:   'testdb'
 	}
-	
+
 	// Encode to heroscript
 	encoded := encode[PostgresqlClient](original)!
 
@@ -78,10 +76,10 @@ fn test_postgres_client_encode_decode_roundtrip() ! {
 	// if true {
 	// 	panic("sss")
 	// }
-	
+
 	// Decode back from heroscript
 	decoded := decode[PostgresqlClient](encoded)!
-	
+
 	// Verify roundtrip
 	assert decoded.name == original.name
 	assert decoded.user == original.user
@@ -95,35 +93,35 @@ fn test_postgres_client_encode() ! {
 	// Test encoding with different configurations
 	test_cases := [
 		PostgresqlClient{
-			name: 'minimal'
-			user: 'root'
-			port: 5432
-			host: 'localhost'
+			name:     'minimal'
+			user:     'root'
+			port:     5432
+			host:     'localhost'
 			password: ''
-			dbname: 'postgres'
+			dbname:   'postgres'
 		},
 		PostgresqlClient{
-			name: 'full_config'
-			user: 'admin'
-			port: 5433
-			host: 'remote.server.com'
+			name:     'full_config'
+			user:     'admin'
+			port:     5433
+			host:     'remote.server.com'
 			password: 'securepass'
-			dbname: 'production'
+			dbname:   'production'
 		},
 		PostgresqlClient{
-			name: 'localhost_dev'
-			user: 'dev'
-			port: 5432
-			host: '127.0.0.1'
+			name:     'localhost_dev'
+			user:     'dev'
+			port:     5432
+			host:     '127.0.0.1'
 			password: 'devpassword'
-			dbname: 'devdb'
-		}
+			dbname:   'devdb'
+		},
 	]
-	
+
 	for client in test_cases {
 		encoded := encode[PostgresqlClient](client)!
 		decoded := decode[PostgresqlClient](encoded)!
-		
+
 		assert decoded.name == client.name
 		assert decoded.user == client.user
 		assert decoded.port == client.port
@@ -134,50 +132,50 @@ fn test_postgres_client_encode() ! {
 }
 
 // Play script for interactive testing
-const play_script = "
+const play_script = '
 # PostgresqlClient Encode/Decode Play Script
 # This script demonstrates encoding and decoding PostgresqlClient configurations
 
-!!postgresql_client.configure name:playground user:play_user 
-		port:5432 
-		host:localhost 
-		password:playpass 
+!!define.postgresql_client name:playground user:play_user
+		port:5432
+		host:localhost
+		password:playpass
 		dbname:playdb
 
 # You can also use partial configurations
-!!postgresql_client.configure name:quick_test host:127.0.0.1
+!!define.postgresql_client name:quick_test host:127.0.0.1
 
 # Default configuration (all defaults)
-!!postgresql_client.configure
-"
+!!define.postgresql_client
+'
 
 fn test_play_script() ! {
 	// Test the play script with multiple configurations
 	lines := play_script.split_into_lines().filter(fn (line string) bool {
 		return line.trim(' ') != '' && !line.starts_with('#')
 	})
-	
+
 	mut clients := []PostgresqlClient{}
-	
+
 	for line in lines {
-		if line.starts_with('!!postgresql_client.configure') {
+		if line.starts_with('!!define.postgresql_client') {
 			client := decode[PostgresqlClient](line)!
 			clients << client
 		}
 	}
-	
+
 	assert clients.len == 3
-	
+
 	// First client: full configuration
 	assert clients[0].name == 'playground'
 	assert clients[0].user == 'play_user'
 	assert clients[0].port == 5432
-	
+
 	// Second client: partial configuration
 	assert clients[1].name == 'quick_test'
 	assert clients[1].host == '127.0.0.1'
 	assert clients[1].user == 'root' // default
-	
+
 	// Third client: defaults only
 	assert clients[2].name == 'default'
 	assert clients[2].host == 'localhost'
@@ -188,48 +186,48 @@ fn test_play_script() ! {
 pub fn run_play_script() ! {
 	println('=== PostgresqlClient Encode/Decode Play Script ===')
 	println('Testing encoding and decoding of PostgresqlClient configurations...')
-	
+
 	// Test 1: Basic encoding
 	println('\n1. Testing basic encoding...')
 	client := PostgresqlClient{
-		name: 'example'
-		user: 'example_user'
-		port: 5432
-		host: 'example.com'
+		name:     'example'
+		user:     'example_user'
+		port:     5432
+		host:     'example.com'
 		password: 'example_pass'
-		dbname: 'example_db'
+		dbname:   'example_db'
 	}
-	
+
 	encoded := encode[PostgresqlClient](client)!
 	println('Encoded: ${encoded}')
-	
+
 	decoded := decode[PostgresqlClient](encoded)!
 	println('Decoded name: ${decoded.name}')
 	println('Decoded host: ${decoded.host}')
-	
+
 	// Test 2: Play script
 	println('\n2. Testing play script...')
 	test_play_script()!
 	println('Play script test passed!')
-	
+
 	// Test 3: Edge cases
 	println('\n3. Testing edge cases...')
 	edge_client := PostgresqlClient{
-		name: 'edge'
-		user: ''
-		port: 0
-		host: ''
+		name:     'edge'
+		user:     ''
+		port:     0
+		host:     ''
 		password: ''
-		dbname: ''
+		dbname:   ''
 	}
-	
+
 	edge_encoded := encode[PostgresqlClient](edge_client)!
 	edge_decoded := decode[PostgresqlClient](edge_encoded)!
-	
+
 	assert edge_decoded.name == 'edge'
 	assert edge_decoded.user == ''
 	assert edge_decoded.port == 0
 	println('Edge cases test passed!')
-	
+
 	println('\n=== All tests completed successfully! ===')
-}
\ No newline at end of file
+}
diff --git a/lib/data/paramsparser/params_reflection.v b/lib/data/paramsparser/params_reflection.v
index a95e6662..5b5dbd32 100644
--- a/lib/data/paramsparser/params_reflection.v
+++ b/lib/data/paramsparser/params_reflection.v
@@ -107,99 +107,110 @@ pub fn encode[T](t T, args EncodeArgs) !Params {
 	// struct_attrs := attrs_get_reflection(mytype)
 
 	$for field in T.fields {
-		val := t.$(field.name)
-		field_attrs := attrs_get(field.attrs)
-		mut key := field.name
-		if 'alias' in field_attrs {
-			key = field_attrs['alias']
+		// Check if field has skip attribute
+		mut should_skip := false
+		for attr in field.attrs {
+			if attr.contains('skip') {
+				should_skip = true
+				break
+			}
 		}
-		$if field.is_option {
-			// Handle optional fields
-			if val != none {
-				// Unwrap the optional value before type checking and encoding
-				// Get the unwrapped value using reflection
-				// This is a workaround for V's reflection limitations with optionals
-				// We assume that if val != none, then it can be safely unwrapped
-				// and its underlying type can be determined.
-				// This might require a more robust way to get the underlying value
-				// if V's reflection doesn't provide a direct 'unwrap' for generic `val`.
-				// For now, we'll rely on the type checks below.
-				// The `val` here is the actual value of the field, which is `?T`.
-				// We need to check the type of `field.typ` to know what `T` is.
 
-				// Revert to simpler handling for optional fields
-				// Rely on V's string interpolation for optional types
-				// If val is none, this block will be skipped.
-				// If val is not none, it will be converted to string.
-				params.set(key, '${val}')
+		if !should_skip {
+			val := t.$(field.name)
+			field_attrs := attrs_get(field.attrs)
+			mut key := field.name
+			if 'alias' in field_attrs {
+				key = field_attrs['alias']
 			}
-		} $else $if val is string || val is int || val is bool || val is i64 || val is u32
-			|| val is time.Time || val is ourtime.OurTime {
-			params.set(key, '${val}')
-		} $else $if field.is_enum {
-			params.set(key, '${int(val)}')
-		} $else $if field.typ is []string {
-			mut v2 := ''
-			for i in val {
-				if i.contains(' ') {
-					v2 += "\"${i}\","
-				} else {
-					v2 += '${i},'
-				}
-			}
-			v2 = v2.trim(',')
-			params.params << Param{
-				key:   field.name
-				value: v2
-			}
-		} $else $if field.typ is []int {
-			mut v2 := ''
-			for i in val {
-				v2 += '${i},'
-			}
-			v2 = v2.trim(',')
-			params.params << Param{
-				key:   field.name
-				value: v2
-			}
-		} $else $if field.typ is []bool {
-			mut v2 := ''
-			for i in val {
-				v2 += '${i},'
-			}
-			v2 = v2.trim(',')
-			params.params << Param{
-				key:   field.name
-				value: v2
-			}
-		} $else $if field.typ is []u32 {
-			mut v2 := ''
-			for i in val {
-				v2 += '${i},'
-			}
-			v2 = v2.trim(',')
-			params.params << Param{
-				key:   field.name
-				value: v2
-			}
-		} $else $if field.typ is $struct {
-			// TODO: Handle embeds better
-			is_embed := field.name[0].is_capital()
-			if is_embed {
-				$if val is string || val is int || val is bool || val is i64 || val is u32
-					|| val is time.Time {
+			$if field.is_option {
+				// Handle optional fields
+				if val != none {
+					// Unwrap the optional value before type checking and encoding
+					// Get the unwrapped value using reflection
+					// This is a workaround for V's reflection limitations with optionals
+					// We assume that if val != none, then it can be safely unwrapped
+					// and its underlying type can be determined.
+					// This might require a more robust way to get the underlying value
+					// if V's reflection doesn't provide a direct 'unwrap' for generic `val`.
+					// For now, we'll rely on the type checks below.
+					// The `val` here is the actual value of the field, which is `?T`.
+					// We need to check the type of `field.typ` to know what `T` is.
+
+					// Revert to simpler handling for optional fields
+					// Rely on V's string interpolation for optional types
+					// If val is none, this block will be skipped.
+					// If val is not none, it will be converted to string.
 					params.set(key, '${val}')
 				}
-			} else {
-				if args.recursive {
-					child_params := encode(val)!
-					params.params << Param{
-						key:   field.name
-						value: child_params.export()
+			} $else $if val is string || val is int || val is bool || val is i64 || val is u32
+				|| val is time.Time || val is ourtime.OurTime {
+				params.set(key, '${val}')
+			} $else $if field.is_enum {
+				params.set(key, '${int(val)}')
+			} $else $if field.typ is []string {
+				mut v2 := ''
+				for i in val {
+					if i.contains(' ') {
+						v2 += "\"${i}\","
+					} else {
+						v2 += '${i},'
 					}
 				}
+				v2 = v2.trim(',')
+				params.params << Param{
+					key:   field.name
+					value: v2
+				}
+			} $else $if field.typ is []int {
+				mut v2 := ''
+				for i in val {
+					v2 += '${i},'
+				}
+				v2 = v2.trim(',')
+				params.params << Param{
+					key:   field.name
+					value: v2
+				}
+			} $else $if field.typ is []bool {
+				mut v2 := ''
+				for i in val {
+					v2 += '${i},'
+				}
+				v2 = v2.trim(',')
+				params.params << Param{
+					key:   field.name
+					value: v2
+				}
+			} $else $if field.typ is []u32 {
+				mut v2 := ''
+				for i in val {
+					v2 += '${i},'
+				}
+				v2 = v2.trim(',')
+				params.params << Param{
+					key:   field.name
+					value: v2
+				}
+			} $else $if field.typ is $struct {
+				// TODO: Handle embeds better
+				is_embed := field.name[0].is_capital()
+				if is_embed {
+					$if val is string || val is int || val is bool || val is i64 || val is u32
+						|| val is time.Time {
+						params.set(key, '${val}')
+					}
+				} else {
+					if args.recursive {
+						child_params := encode(val)!
+						params.params << Param{
+							key:   field.name
+							value: child_params.export()
+						}
+					}
+				}
+			} $else {
 			}
-		} $else {
 		}
 	}
 	return params
diff --git a/test_basic.vsh b/test_basic.vsh
index c5d9a013..1e1646bf 100755
--- a/test_basic.vsh
+++ b/test_basic.vsh
@@ -202,8 +202,6 @@ generate_test.v
 dbfs_test.v
 namedb_test.v
 timetools_test.v
-encoderhero/encoder_test.v
-encoderhero/decoder_test.v
 code/codeparser
 gittools_test.v
 link_def_test.v