Add --note flag to associate messages with frames #383
Conversation
* Made 'watson.frames.Frames' a subclass of 'collections.OrderedDict'. * Changed order of items in 'watson.frames.Frame' to be more consistent with 'Frames.add()' and 'Frames.new_frame()'. This changes the on-disk JSON representation of the frames, i.e. the format of the '<app dir>/watson/frames' file. The code handles loading the old format automatically and on saving the file gets written in the new format. * Removed ability to look up frame by index with dict look-up syntax and provide 'Frames.get_by_index()' method as replacement. * Removed ability to delete frame by index (was not used aywhere). * Removed ability to get Frames column values with dict look-up syntax and provide 'Frames.get_column()' generator as replacement. * Iterating over 'Watson.frames' yields frame IDs instead of 'Frame' instances. Where the latter is desired, the 'values()' method must be used. * Adding a frame by assigning a 'Frame' instance to a key of a `Frames`, makes a copy of the 'Frame' instance with the given key as the frame ID. * Made 'Frame.filter()' a generator. * Adapted code using 'Frame' and 'Frames' to these changes. * Adapted tests and test data files. + Command line interface remains unchanged.
…ame object Signed-off-by: Christopher Arndt <chris@chrisarndt.de>
…mes format too; Clarify docstring on passing frames to construcutor as well Signed-off-by: Christopher Arndt <chris@chrisarndt.de>
Signed-off-by: Christopher Arndt <chris@chrisarndt.de>
Signed-off-by: Christopher Arndt <chris@chrisarndt.de>
…der for better perfomance in most likely case Signed-off-by: Christopher Arndt <chris@chrisarndt.de>
Signed-off-by: Christopher Arndt <chris@chrisarndt.de>
Signed-off-by: Christopher Arndt <chris@chrisarndt.de>
Signed-off-by: Christopher Arndt <chris@chrisarndt.de>
Signed-off-by: Christopher Arndt <chris@chrisarndt.de>
Signed-off-by: Christopher Arndt <chris@chrisarndt.de>
Signed-off-by: Christopher Arndt <chris@chrisarndt.de>
…evelopment)" This reverts commit c7c3c84.
…verse order for better perfomance in most likely case" This reverts commit 8d2206f.
This reverts commit c956089.
This reverts commit 73c2593.
…ist items" This reverts commit 714b581.
… new Frames format too; Clarify docstring on passing frames to construcutor as well" This reverts commit 553e34a.
…to the same object" This reverts commit 8ed29c8.
Signed-off-by: Christopher Arndt <chris@chrisarndt.de> Merge branch 'feature/log-message' of github.com:SpotlightKid/Watson into feature/log-message Signed-off-by: Christopher Arndt <chris@chrisarndt.de>
…ex'" This reverts commit a1a905a.
This reverts commit 92b8733.
…_log_message # Conflicts: # tests/test_watson.py # watson.completion # watson/cli.py
-m and -M are currently used by some commands. -n and -N aren't. This diverges from the git -m/--message naming we were copying, but that doesn't bother me too much. The parameter behaves slightly different in watson, so it might even be better that we don't reuse a commonly used parameter name in another tool, as not to cause confusion
Without this, the note is sometimes interpreted as `updated_at` which throws an error
| ) | ||
| for frame in frames | ||
| )) | ||
| for frame in frames: |
There was a problem hiding this comment.
I was not sure how to incorporate the if-statement in this section, so this is the only place in the merge where I went with the older syntax with the traditional for loop. Let me know if you prefer the have this change reverse and how it would be written with the 'note' if statement below.
This is also what causes the flake8 test to fail, so please advice on what to do here.
| def format_note(note): | ||
| return u"{}{}".format( | ||
| style('note', '>> '), | ||
| style('note', note.replace('\n', '\n' + ' '*20)) |
There was a problem hiding this comment.
This aligns rows of notes containing \n in the log output.
| if note is None: | ||
| note = old.get('note') | ||
| elif old.get('note') is not None: | ||
| print('Overwriting old note:\n>> {}'.format(old.get('note'))) |
There was a problem hiding this comment.
This means that this warning is printed first, before the always printed watson stop message. This is a bit visually jarring, but it might be a good thing since it is an important thing to note, see screenshot. I tried passing a variable up to watson cli and then use echo after the stop message, but could not find a good way of doing this since the frameattributesare actually properties are note modifieable so I could not pass say a tuple and then print the old note and overwrite the property with the new note.
|
@k4nar Just a reminder to have a look at this when you get a chance. |
|
@k4nar and @jmaupetit I have been running this branch locally since opening the PR and it seems to be working well (although I don't use notes that often tbh). Just a reminder to review this PR when you have time. |
|
Just a note that I didn't use this feature much and I am no longer running this branch locally. I ran into some issues with my frames file format when I moved it to a new watson installed from the main branch, so I went ahead and deleted the notes fields for each frame and it is now working again. I didn't have time to investigate exactly why this issue occurred, but thought I would leave a note that there might be incompatibilities with newer watson versions in case others are considering running from this PR (you probably should wait until when/if this feature is merged into main). |
@jmaupetit This is a continuation of #337. As mentioned there, I merged master into this branch because the rebase was a headache with so many conflicting changes (possible I'm not doing this optimally). I suspect this doesn't matter since you usually squash when merging, but it would be great if this was given priority over other PR's, since it is hard to rebase if master is updated.
I added @k4nar 's suggestions in the three first commits. Just a comment that although the plural form
--notesis more natural for log since it is short forshort_notes, it might be confusing since it is--noteeverywhere else. I'll let you decide what to do, if anything.I added three commits of my own, only 4e0e4b6 is a functional change. Without it, I would not be able to edit frames since the new
notefield would be interpreted as theupdated_atfield andwatson editraised an error trying to convert this to a date. I have added this and a few more comments on the code also.Close #337 #252 #93