@rkashinin

Thanks for the review,
I think that you are right and in this situation json_name doesn’t matter, but there is no guarantee that json_name and layout_json always equal. That’s why without json_name now, it will be much more difficult to change the program later.

From my point of view it's a matter of design but the setitem function sets the element in the dictionary. So I don’t really understand, why do you prefer square brackets?

@rkashinin

I think that you are right and in this situation json_name doesn’t matter, but there is no guarantee that json_name and layout_json always equal. That’s why without json_name now, it will be much more difficult to change the program later.

Sounds reasonable.

As for __setitem__(). There are several way to set item for specific key in dictionary: subscript notation, using update() method and using __setitem__() method (like you've used).
Meanwhile calling of __setitem__() entails poor perfomance time comparing to subscript notation:

>>> import timeit
>>> using_brackets = """
... bar = {}
... bar['foo'] = 'foo'
... """
>>> using_setitem = """
... bar = {}
... bar.__setitem__('foo', 'foo')
... """
>>> print("Brackets: ", timeit.timeit(using_brackets, number=10000))
>>> print("__setitem__(): ", timeit.timeit(using_setitem, number=10000))
Brackets:  0.000403950998588698
__setitem__():  0.0008579820023442153

(Python 3.8.10 \n[GCC 9.4.0])

Not to mention it's essentially less readable form of doing the same thing. Similar points are described at this topic on SO.

Futhermore, Python's special method __setitem__() is supposed to implement behaviour when using subscript notation:

class CustomDict(dict):
    def __setitem__(self, k, v):
        print(f'Set value "{v}" for key "{k}"')
        super().__setitem__(k, v)
        
>>> mydict = CustomDict()
>>> mydict['bar'] = 'foo'
Set value "foo" for key "bar"
>>> print(mydict)
{'bar': 'foo'}

(Python 3.8.10 \n[GCC 9.4.0])
So in this case it looks like unreasonable calling of Python's special method.

0001-Fix-for-issue-676-title-field-not-seem-to-be-working.patch
0002-Fix-for-issue-676-title-field-not-seem-to-be-working.patch
Thanks for your explanation,
According to your review I’ve made some changes and I send you patch files with them.
Is it ok, or should I make another pull request?

@rkashinin thanks for the update!

You can just upload a new commit in a top of the current. Then it's going to be automatically updated here, in the current PR.
So, it would be like this:

1. Open your project, ensure you're in the right branch (issue-676 in your case): git checkout issue-676;
2. Make changes to code;
3. Make a new commit: git add terminatorlib/configjson.py, then git commit and a message like minor fix;
4. Upload then: git push -u origin issue-676;

(Note: the argument "origin" in 4th step may be different depending on which settings are in your .git/config file; to clarify you can run git remote -v and chose the name which corresponds to URL of your fork on GitHub)

Alternatively you can create a new pull request, of course, it's up to you, choose the easier option :)

Hope this help!

Thanks for your advice,
I wasn’t sure about what to do, couse I’m a new one on git hub.
You helped a lot!