Copy link Copy Markdown

Member

Since people are confused about ADD working as modify, lets make it more clear

Copy link Copy Markdown

Member

Dunno if i like this idea, but at least document that ADD does the same thing if the network already exists, instead of removing that line completely.

Also MODIFY should fail if the network/server/channel doesn't exist instead of being exactly the same thing as add

Copy link Copy Markdown

Member Author

Yeah, I was thinking about that, but since it is just an alias to make it more clear to users I thought the duplication of code would be more work than warranted
But sure, I can add cmd_server_modify & co if you want that.
Or I can amend the help files with info that modify is just an alias for add

Copy link Copy Markdown

Member

I'm not fully convinced about this. I think this is a documentation issue.

Copy link Copy Markdown

Member Author

It is a documentation issue also, but adding modify doesn't add much code and it would make it more clear to users what to do. This PR is mostly help modifications but it's easy to overlook documentation when you are looking for a particular thing.

Copy link Copy Markdown

Contributor

I'm not very much in favour of this unless you actually modify MODIFY to disable its ADD functionality. (I'd keep ADD as fully functional modify for backward compat)

Copy link Copy Markdown

Contributor

I like the direction your code is taking, however you will need to refactor the common code parts into static methods instead of using copypasta

…still add/modify

Copy link Copy Markdown

Contributor

hi @vague666 , I really like the change you're proposing even if it is conceptually minor. however there is still work left before I can accept it. You know what I've been thinking about may be better instead:

take the existing _add functions, rename them to from cmd_..._add to ..._add_modify with a final argument gboolean add
only slightly modify them each, changing if (rec == NULL) { to if (rec == null && !add) { error out } else if (rec == NULL && add) { original code goes on
mask each line that should not be applicable to modify in such a way
create cmd_..._add and cmd_..._modify that call ..._add_modify(..., true/false)

That would also immediately get rid of some of the code duplication I marked with A and B

		g_free_and_null(rec->own_host);
		rec->own_ip4 = rec->own_ip6 = NULL;
		}

Choose a reason for hiding this comment