Draft: Some improvements #20

liferooter · 2021-12-17T14:57:04Z

Some code and mechanism improvements

Prince781 · 2021-12-18T16:19:34Z

Thanks for your efforts, @liferooter . I'll take a look at this soon when I'm done with finals.
One thing you should keep in mind is that I wrote this code to be compatible with Vala 0.48 and Ubuntu 20.04 / eOS 6. If you're changing things, please make sure that Valdo is still compatible with those OSes. For example, null-safe member accesses are nice but are too recent in Vala for me to use.

liferooter · 2021-12-18T16:22:36Z

If you're changing things, please make sure that Valdo is still compatible with those OSes.

Okay, I'll try to keep compability and test on these OSes

Prince781 · 2021-12-28T16:13:10Z

@liferooter You should try to get this stuff merged soon. I think I'd prefer if you sent me multiple PRs, that way I can evaluate your changes more thoroughly and merge them sooner before merge conflicts arise.

liferooter · 2021-12-28T16:44:43Z

You should try to get this stuff merged soon.

Okay, I'll finish today

Commonly used flag to print application version is --version in long form and -v in short form, not -V

Empty array as array terminator is not hack, it's correct way to implement such thing.

- Move project version from version.vala to config.vala - Delete version.vala

liferooter · 2021-12-28T18:02:34Z

@Prince781 now PR has no conflicts and ready for review and merging

Prince781 · 2021-12-28T20:49:48Z

A couple of things:

Running valdo without any arguments should show a list of all available templates. You must not remove this behavior.
Deserializing the template JSON fails on Ubuntu 20.04:

ubuntu@ubuntu:~/dev/valdo$ valdo -l

(valdo:3132): Json-WARNING **: 15:46:29.593: Failed to deserialize "description" property of type "(null)" for an object of type "ValdoTemplate"

(valdo:3132): Json-WARNING **: 15:46:29.593: Failed to deserialize "description" property of type "(null)" for an object of type "ValdoTemplate"

(valdo:3132): Json-WARNING **: 15:46:29.593: Failed to deserialize "description" property of type "(null)" for an object of type "ValdoTemplate"

(valdo:3132): Json-WARNING **: 15:46:29.593: Failed to deserialize "description" property of type "(null)" for an object of type "ValdoTemplate"

(valdo:3132): Json-WARNING **: 15:46:29.593: Failed to deserialize "description" property of type "(null)" for an object of type "ValdoTemplate"
Available templates:
--------------------
lib         - (null)
gtk         - (null)
new         - (null)
swifty-gtk4 - (null)
eos         - (null)
ubuntu@ubuntu:~/dev/valdo$

liferooter · 2021-12-28T21:14:10Z

Running valdo without any arguments should show a list of all available templates. You must not remove this behavior.

I think that such behavior is non-typical and non-intuitive, doesn't it?

Prince781

Reviewed. Multiple changes needed. You also need to rename the files back to have the .in suffix. I explained why in one of my review comments.

src/expression.vala

src/main.vala

src/templateengine.vala

templates/eos/template.json

templates/gtk/template.json

templates/lib/template.json

templates/new/template.json

templates/swifty-gtk4/template.json

Prince781 · 2021-12-28T21:23:01Z

I think that such behavior is non-typical and non-intuitive, doesn't it?

It's perfectly intuitive—the user sees all available templates if he/she didn't request one. There's no rule that says you can't do this. And if you want to talk about other programs that do a similar thing, dotnet new does this too.

Prince781 · 2021-12-28T23:03:21Z

I think the rest of the code is pretty well-organized. I wrote this quickly and so it's nice to see someone taking the time to make the code look nice. 😃

liferooter · 2021-12-29T10:03:43Z

2. Deserializing the template JSON fails on Ubuntu 20.04:

ubuntu@ubuntu:~/dev/valdo$ valdo -l

(valdo:3132): Json-WARNING **: 15:46:29.593: Failed to deserialize "description" property of type "(null)" for an object of type "ValdoTemplate"

(valdo:3132): Json-WARNING **: 15:46:29.593: Failed to deserialize "description" property of type "(null)" for an object of type "ValdoTemplate"

(valdo:3132): Json-WARNING **: 15:46:29.593: Failed to deserialize "description" property of type "(null)" for an object of type "ValdoTemplate"

(valdo:3132): Json-WARNING **: 15:46:29.593: Failed to deserialize "description" property of type "(null)" for an object of type "ValdoTemplate"

(valdo:3132): Json-WARNING **: 15:46:29.593: Failed to deserialize "description" property of type "(null)" for an object of type "ValdoTemplate"
Available templates:
--------------------
lib         - (null)
gtk         - (null)
new         - (null)
swifty-gtk4 - (null)
eos         - (null)
ubuntu@ubuntu:~/dev/valdo$

Just reproduces this bug in Ubuntu 20.04 VM. Have no idea what's wrong and how to fix it without manuary reading every property of every class. But all works on Elementary OS 6.1 with Vala 0.48.20 and there will be new Ubuntu LTS soon with even newer Vala. So may I spit on compatibility with too old Ubuntu which requires to make the app don't use some really needed Vala features?

Prince781 · 2021-12-29T17:40:41Z

But all works on Elementary OS 6.1 with Vala 0.48.20 and there will be new Ubuntu LTS soon with even newer Vala. So may I spit on compatibility with too old Ubuntu which requires to make the app don't use some really needed Vala features?

Yes, but then you must change meson.build to require json-glib >= 1.5.2

Prince781

Looks good to me.

Prince781 · 2021-12-30T23:17:32Z

I had to keep a workaround for JSON-GLib 1.4.4 because eOS 7 won't come out for another 5-6 months and this package won't be updated for the current release.

Prince781 · 2021-12-30T23:20:59Z

@liferooter merged. Thanks a lot!

liferooter changed the title ~~Draft: A lof of improvements~~ Draft: Some improvements Dec 18, 2021

liferooter marked this pull request as ready for review December 28, 2021 17:53

liferooter added 24 commits December 28, 2021 20:58

Use -v instead of -V for to print version

60e79d7

Commonly used flag to print application version is --version in long form and -v in short form, not -V

Remove issue link since issue is closed

3629734

Empty array as array terminator is not hack, it's correct way to implement such thing.

Add more docs for command-line options

6729551

Use upper-case for constants

1ae9078

Don't use explicit prefixes for enums

c3fb605

Remove email regex from config

89294b6

Simplify config.vala

ddaf452

- Move project version from version.vala to config.vala - Delete version.vala

Align option entries to improve readability

4c241cf

Use null instead of 0 to define null string

addf425

Move ctx.add_main_entries out of try block

bd968da

Add --list-templates/-l option to list available templates

eab5d82

Change --version flag description

405379a

Move app name retrieved from args to variable

6cdb588

Remove option documentation since they have descriptions

0171f19

Replace forgotted args[0] occurences with app_name

ef61a0a

Don't use 0 instead of NONE flag

40ba4b6

Simplify list_templates function

a3dda75

Use option_ prefix for option variables

dc940ea

Place main.vala code into Valdo.Main namespace

0f14268

Sort content of main.vala to improve readability

a652c6d

Improve and simplify main.vala code

b59512c

Add some docs to main.vala

bac735a

Simplify JSON deserialization

3a782a2

Remove meaningless fixme and follow Elementary code style

02fbb7f

liferooter added 3 commits December 28, 2021 21:00

Add documentation for all functions

35549a4

Fix variable expansion

15a2129

Add .vscode to .gitignore

3ec337d

liferooter force-pushed the wip/liferooter/a-lot-of-improvements branch from cd0e206 to 3ec337d Compare December 28, 2021 18:01

Prince781 requested changes Dec 28, 2021

View reviewed changes

Avoid use replace_eval when possible

4a7e08c

List available templates when runs without args

e1a4e7b

Bring back list of template files in metadata

e444af9

liferooter requested a review from Prince781 December 30, 2021 06:00

Prince781 approved these changes Dec 30, 2021

View reviewed changes

Prince781 added 7 commits December 30, 2021 23:12

Avoid initializers for zero-initialized properties

83cf0bc

Expand variables after evaluating default values.

7e347de

Keep enum prefix where the meaning isn't obvious

fed5078

Don't use git config fields if empty

091eb66

Fix doc comment at top of expression.vala

7966083

Add Gleb Smirnov to copyright

a790de4

Fix error message when template is not found

63765e2

Prince781 force-pushed the wip/liferooter/a-lot-of-improvements branch from 48390c9 to ec6ba4e Compare December 30, 2021 23:14

Support json-glib 1.4.4 for eOS 6.1

dacc7fb

Prince781 force-pushed the wip/liferooter/a-lot-of-improvements branch from ec6ba4e to dacc7fb Compare December 30, 2021 23:19

Prince781 merged commit f3959de into vala-lang:master Dec 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft: Some improvements #20

Draft: Some improvements #20

liferooter commented Dec 17, 2021 •

edited

Loading

Prince781 commented Dec 18, 2021

liferooter commented Dec 18, 2021

Prince781 commented Dec 28, 2021 •

edited

Loading

liferooter commented Dec 28, 2021

liferooter commented Dec 28, 2021

Prince781 commented Dec 28, 2021

liferooter commented Dec 28, 2021 •

edited

Loading

Prince781 left a comment

Prince781 commented Dec 28, 2021

Prince781 commented Dec 28, 2021

liferooter commented Dec 29, 2021

Prince781 commented Dec 29, 2021

Prince781 left a comment

Prince781 commented Dec 30, 2021

Prince781 commented Dec 30, 2021

Draft: Some improvements #20

Draft: Some improvements #20

Conversation

liferooter commented Dec 17, 2021 • edited Loading

Prince781 commented Dec 18, 2021

liferooter commented Dec 18, 2021

Prince781 commented Dec 28, 2021 • edited Loading

liferooter commented Dec 28, 2021

liferooter commented Dec 28, 2021

Prince781 commented Dec 28, 2021

liferooter commented Dec 28, 2021 • edited Loading

Prince781 left a comment

Choose a reason for hiding this comment

Prince781 commented Dec 28, 2021

Prince781 commented Dec 28, 2021

liferooter commented Dec 29, 2021

Prince781 commented Dec 29, 2021

Prince781 left a comment

Choose a reason for hiding this comment

Prince781 commented Dec 30, 2021

Prince781 commented Dec 30, 2021

liferooter commented Dec 17, 2021 •

edited

Loading

Prince781 commented Dec 28, 2021 •

edited

Loading

liferooter commented Dec 28, 2021 •

edited

Loading