Implement a way to edit the last entry. #247

icemac · 2023-11-30T13:55:40Z

Fixes #160.

sallner

Looks good from my side.

mgedmin

I like this! With some minor suggestions.

I could merge it as-is and then fix up the things I want done differently directly in git master. Should I do that? I would like to hear your opinion about e.g. commenting out entries instead of removing them entirely, to avoid possible loss of data.

mgedmin · 2023-12-01T11:52:55Z

src/gtimelog/main.py

+        if text is not None:
+            window.task_entry.set_text(text)
+        window.task_entry.grab_focus()
+        window.task_entry.select_region(-1, -1)


Should this be an app-global action rather than a window action?

I wonder if we should do something special if the user is currently engaged in history browsing, like jump to the current day when you do this?

Currently if you're history browsing and enter something in the log enter, when you press Enter, the view returns back to today. I think the same thing should happen if you press Ctrl+Backspace.

Should this be an app-global action rather than a window action?

I do not know what's the difference. Does app global mean it gets an entry in the menu?

gocept.gtimelog had no history browsing. So I did not consider it during the implementation. I think if previous days cannot be edited (= adding entries), deletion should not happen, when the user is not looking at the current day. Removing the last entry from the current day when looking at a different one, I'd consider unexpected.

I do not know what's the difference. Does app global mean it gets an entry in the menu?

No, this is a purely internal distinction. Window actions get dispatched to the current window, and seeing as the first thing you do in the global action is find the current window to do things with its widgets, this seems like it would be more natural to move the handler method into the window subclass.

I think if previous days cannot be edited (= adding entries), deletion should not happen, when the user is not looking at the current day. Removing the last entry from the current day when looking at a different one, I'd consider unexpected.

Yeah, that makes sense.

mgedmin · 2023-12-01T11:55:43Z

src/gtimelog/timelog.py

+            # remove line which divides days if necessary
+            lines = lines[:-1]
+        with open(self.filename, "w", encoding='utf-8') as f:
+            f.write(''.join(lines))


I wonder if f.writelines(lines) would be nicer here?

It would probably be nicer and more performant.

mgedmin · 2023-12-01T12:01:20Z

CHANGES.rst

@@ -40,6 +40,8 @@ Changelog
 - Grouped task entries can now be sorted by start date, name, duration or
  according to tasks.txt order (GH: #228).

+- Add the ability to change the last entry using Primary+BackSpace (GH: #247).


I find the GTK terminology of Primary+... confusing (and I think they dropped it for GTK 4?) so I would suggest

Suggested change

- Add the ability to change the last entry using Primary+BackSpace (GH: #247).

- Add the ability to change the last entry using Ctrl+Backspace (GH: #247).

instead

On MacOS it is actually Command-Backspace, so I wrote primary. But I am also fine with Ctrl.

Hm, MacOS was probably why Gtk introduced those Primary+ etc. modifiers. That's fine then.

mgedmin · 2023-12-01T12:07:45Z

src/gtimelog/timelog.py

+        lines = lines[:-1]
+        if not lines[-1].strip():
+            # remove line which divides days if necessary
+            lines = lines[:-1]


My greatest worry about this feature is that there's no undo if you (accidentally) press Ctrl+Backspace more than once in a row. Maybe instead of removing the line from the file it would be better to comment it out? Then at least you can recover with a text editor.

The timelog.txt format supports comments using # at the beginning of the line. I plan to use some of the comments for directives that set things like timezones or virtual midnight times, so maybe we should use ## for commented-out entries.

I have been using this feature without undo for years in gocept.gtimelog. Yes, it happened to me that I called the command too often and thus loosing data. But I was usually able to reconstruct from memory, when it happened. (Mostly I use this feature when I added an entry because I thought I was done with a task, but then worked for some minutes on it to really complete it.)

So I see no immediate need for an undo ability. If it is too dangerous for some users, then they still can do it the old way be editing the timelog.txt file.

The alternative could be, to do nothing when hitting the key combination and there is text in the field where you enter your time log entries.

The alternative could be, to do nothing when hitting the key combination and there is text in the field where you enter your time log entries.

I like this!

icemac · 2023-12-15T15:13:53Z

I like this! With some minor suggestions.

I like that you like it. 😀

I could merge it as-is and then fix up the things I want done differently directly in git master. Should I do that? I would like to hear your opinion about e.g. commenting out entries instead of removing them entirely, to avoid possible loss of data.

Feel free to merge and to improve afterwards.

icemac · 2024-01-29T13:10:56Z

I am using this branch in my daily work for over a month now. I think it is good enough to be merged.

mgedmin · 2024-01-30T12:46:39Z

Amazingly, I discovered the biggest flaw only after merging this: Ctrl-Backspace, by default, is used to delete word backwards in text entry boxes. And I use it a lot.

Luckily, this discovery happened after I already changed my copy of the code to comment out the entry instead of removing it.

But. I need to find a different shortcut.

mgedmin · 2024-01-30T12:51:28Z

I need to find a different shortcut.

Without much thought I changed it to Ctrl+Shift+Backspace.

mgedmin · 2024-01-30T13:20:23Z

And now I rewrote the implementation to comment out the lines instead of deleting them, as well as made some refactorings. You may want to give it another test, in case I broke something and didn't catch it while testing locally.

mgedmin · 2024-02-02T12:52:06Z

Without much thought I changed it to Ctrl+Shift+Backspace.

Today I reached for this feature for the first time, and my first instinct was to press Ctrl+Shift+Up, like I was editing the last message on Slack.

Hmm.

icemac · 2024-02-02T16:01:56Z

Thank you for merging this branch and doing some adaptions to it.

I have to admin that I am not a big fan of commenting lines instead of removing them as it clutters timelog.txt (yes, I am using this command quite often) and it requires changes in gtimelog2jira and gtimelog2tick (and other tools parsing timelog.txt) to be able to handle comments.

mgedmin · 2024-02-04T11:07:17Z

it requires changes in gtimelog2jira

Ehh? I've had comments in my timelog.txt since a long time ago, and I never noticed gtimelog2jira having problems with that.

Looking at the code, gtimelog2jira uses the same algorithm as gtimelog itself and silently ignores all the lines that don't start with a valid ISO 8601 datetime followed by a colon, so it should work fine.

icemac · 2024-02-06T07:57:09Z

I thought having comments was a new concept. Sorry for the noise, I did not even check gtimelog2tick. As I now found out, it works well with comments.

So now I have less of a problem with commenting out instead of deleting.
Let's just keep it like it is now.

icemac added 3 commits November 30, 2023 14:52

Implement a way to edit the last entry.

dbba5f6

Fixes gtimelog#160.

Add tests to fix coverage.

b11f29c

Add PR number.

a70548e

sallner approved these changes Nov 30, 2023

View reviewed changes

mgedmin approved these changes Dec 1, 2023

View reviewed changes

mgedmin merged commit ad94da1 into gtimelog:master Jan 30, 2024
12 checks passed

icemac deleted the edit-last-entry branch January 31, 2024 07:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement a way to edit the last entry. #247

Implement a way to edit the last entry. #247

icemac commented Nov 30, 2023

sallner left a comment

mgedmin left a comment

mgedmin Dec 1, 2023

mgedmin Dec 1, 2023

icemac Dec 15, 2023

icemac Dec 15, 2023

mgedmin Dec 18, 2023

mgedmin Dec 1, 2023

icemac Dec 15, 2023

mgedmin Dec 1, 2023

icemac Dec 15, 2023

mgedmin Dec 18, 2023

mgedmin Dec 1, 2023

icemac Dec 15, 2023

icemac Dec 15, 2023

mgedmin Dec 18, 2023

icemac commented Dec 15, 2023

icemac commented Jan 29, 2024

mgedmin commented Jan 30, 2024

mgedmin commented Jan 30, 2024

mgedmin commented Jan 30, 2024

mgedmin commented Feb 2, 2024

icemac commented Feb 2, 2024

mgedmin commented Feb 4, 2024

icemac commented Feb 6, 2024

	- Add the ability to change the last entry using Primary+BackSpace (GH: #247).
	- Add the ability to change the last entry using Ctrl+Backspace (GH: #247).

Implement a way to edit the last entry. #247

Implement a way to edit the last entry. #247

Conversation

icemac commented Nov 30, 2023

sallner left a comment

Choose a reason for hiding this comment

mgedmin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

icemac commented Dec 15, 2023

icemac commented Jan 29, 2024

mgedmin commented Jan 30, 2024

mgedmin commented Jan 30, 2024

mgedmin commented Jan 30, 2024

mgedmin commented Feb 2, 2024

icemac commented Feb 2, 2024

mgedmin commented Feb 4, 2024

icemac commented Feb 6, 2024