bastibe / annotate.el

Annotate.el
Other
384 stars 20 forks source link

Added commands to force positioning and color of an annotation #160

Closed cage2 closed 5 months ago

cage2 commented 5 months ago

Hi @bastibe !

How are you?

I have added two commands to set the color or the positioning for each annotation, changes are persistent, as you can guess, this means the the format of the annotation database has changed again :sweat_smile:; By the way the changes are backward compatible, fortunately!

Bye! C.

bastibe commented 5 months ago

Wow, that's quite the change!

I like how you abstracted all the explicit overlay manipulation. Another rung on the ladder of abstractions. Otherwise, looks like a solid change!

I saw there's some intelligence in the annotation placement depending on the line length. IIRC, placement was dependent on the frame width, to push annotations below the line if it doesn't fit in the margin. Does it still do that now that placement is explicit, especially if you change the frame width?

cage2 commented 5 months ago

On Thu, Mar 14, 2024 at 06:15:27AM -0700, you wrote:

Hi!

Wow, that's quite the change!

Thanks, there is a bit of work but they are low hanging fruit because the basic scaffolding was more or less here, at least conceptually.

I like how you abstracted all the explicit overlay manipulation. Another rung on the ladder of abstractions. Otherwise, looks like a solid change!

Thanks, i was afraid i was gone too far with all those one line functions! 😅

I saw there's some intelligence in the annotation placement depending on the line length. IIRC, placement was dependent on the frame width, to push annotations below the line if it doesn't fit in the margin. Does it still do that now that placement is explicit, especially if you change the frame width?

The user can set the positioning on a new line on a margin or just let the code choose for them based on the space left between the line where the annotated text is. This is the reason why i chose to print a message with the current positioning on `annotate-change-annotation-text-position'; because if you change from an explicit mode to the calculated one, the results can be the same and this could lead to confusion, the user can think of a bug or something like that.

Thanks as usual for the review!

Bye! C.

bastibe commented 5 months ago

I wonder if it might be useful to have a fourth placement category "auto".

cage2 commented 5 months ago

On Fri, Mar 15, 2024 at 07:22:07AM -0700, you wrote:

Hi!

I wonder if it might be useful to have a fourth placement category "auto".

Does not ':by-length' has the same meaning? When the 'position' property of the annotation has been set to this value, the annotation's text is placed accordingly to the space left between the text line and the buffer's border. Id there is room, on the margin; on the next line if not.

Bye! C.

bastibe commented 5 months ago

Oh silly me, you are entirely correct. I completely overlooked that 🥸. Sorry!

cage2 commented 5 months ago

On Sat, Mar 16, 2024 at 07:56:30AM -0700, you wrote:

Hi!

Oh silly me, you are entirely correct. I completely overlooked that 🥸. Sorry!

No problem at all bastibe!

Have a nice sunday! 😀 C.

cage2 commented 5 months ago

OK, merging in a minute! :smile: