Edgewall Software
Modify

Opened 5 years ago

Last modified 13 months ago

#11493 new enhancement

Add config option to show changeset files in ticket's comment

Reported by: t2y <tetsuya.morimoto@…> Owned by:
Priority: low Milestone: next-major-releases
Component: ticket system Version: 1.0-stable
Severity: minor Keywords: CommitTicketUpdater
Cc: arkinform@…
Release Notes:
API Changes:

Description

I want to confirm changeset files in comment related to the ticket. It's useful to find a particular file related to the ticket. I made a patch to do this. How does that sound?

[ticket]
commit_ticket_reference_show_files = true

Attachments (7)

show-changeset-files-in-comment.png (18.5 KB ) - added by t2y <tetsuya.morimoto@…> 5 years ago.
show-changeset-files-in-comment.patch (2.3 KB ) - added by t2y <tetsuya.morimoto@…> 5 years ago.
show-changeset-files-in-comment-improved1.patch (6.8 KB ) - added by t2y <tetsuya.morimoto@…> 5 years ago.
show-changeset-files-in-comment-improved1.png.png (18.5 KB ) - added by t2y <tetsuya.morimoto@…> 5 years ago.
show-changeset-files-in-comment-improved2.patch (6.6 KB ) - added by t2y <tetsuya.morimoto@…> 5 years ago.
show-changeset-files-in-comment-improved2.png (18.3 KB ) - added by t2y <tetsuya.morimoto@…> 5 years ago.
Timeline.png (65.7 KB ) - added by Ryan J Ollos 4 years ago.

Download all attachments as: .zip

Change History (30)

Changed 5 years ago by t2y <tetsuya.morimoto@…>

Changed 5 years ago by t2y <tetsuya.morimoto@…>

comment:1 Changed 5 years ago by Ryan J Ollos

Milestone: next-stable-1.0.x1.1.3
Owner: set to Ryan J Ollos
Status: newassigned

It sounds useful. Thanks for the patch!

This appears to be most appropriate for next-dev-1.1.x. If 1.1.2 isn't released soon we can move the changes to that milestone.

comment:2 Changed 5 years ago by Jun Omae

It would be good to specify number of files to show instead of boolean like [timeline] changeset_show_files. If a changeset has too many files, the comment would be noisy.

comment:3 Changed 5 years ago by Jun Omae

Keywords: CommitTicketUpdater added

comment:4 in reply to:  2 ; Changed 5 years ago by t2y <tetsuya.morimoto@…>

Replying to jomae:

If a changeset has too many files, the comment would be noisy.

Right. However, there is 2 cases to use this function.

  1. confirm a particular file without omission in ticket
  2. convenient to check a particular file with personal curiosity

It might be needed 2 options to address each case?

comment:5 in reply to:  4 ; Changed 5 years ago by Jun Omae

Right. However, there is 2 cases to use this function.

  1. confirm a particular file without omission in ticket
  2. convenient to check a particular file with personal curiosity

It might be needed 2 options to address each case?

The [timeline] changeset_show_files option allows you to configure the following, see TracIni#timeline-section.

  • -1 for unlimited
  • 0 for disable (default)
  • > 0 for files to show (if number of files exceeds the limitation, (U+2026) marker will be shown in the last)

For your cases, it would be good to use -1, I think.

Other things;

  • For deleted file, no #file<N> anchor in changeset view. Instead, [source:path/to/file@base_rev path/to/file], e.g. r11930.
  • For copied or moved file, no #file<N> anchor in changeset view. It would be good to show the following, e.g. r11082.
    • [source:path/to/file@rev] (copied from [source:base_path/to/file@base_rev])
    • [source:path/to/file@rev] (moved from [source:base_path/to/file@base_rev])
  • We should check FILE_VIEW permission for each change. e.g. tags/trac-1.0.1/trac/versioncontrol/web_ui/changeset.py@:995-999#L992.

comment:6 in reply to:  5 Changed 5 years ago by t2y <tetsuya.morimoto@…>

Replying to jomae:

The [timeline] changeset_show_files option allows you to configure the following, see TracIni#timeline-section.

I hadn't known the detail. It's good enough.

Other things;

  • For deleted file, no #file<N> anchor in changeset view. Instead, [source:path/to/file@base_rev path/to/file], e.g. r11930.
  • For copied or moved file, no #file<N> anchor in changeset view. It would be good to show the following, e.g. r11082.
    • [source:path/to/file@rev] (copied from [source:base_path/to/file@base_rev])
    • [source:path/to/file@rev] (moved from [source:base_path/to/file@base_rev])
  • We should check FILE_VIEW permission for each change. e.g. tags/trac-1.0.1/trac/versioncontrol/web_ui/changeset.py@:995-999#L992.

Indeed. I'm going to improve my patch soon. Thank you for good advice!

Changed 5 years ago by t2y <tetsuya.morimoto@…>

Changed 5 years ago by t2y <tetsuya.morimoto@…>

comment:7 Changed 5 years ago by t2y <tetsuya.morimoto@…>

I improved previous patch, thanks to jomae (comment:5).

The Screen shot is here.

comment:8 Changed 5 years ago by Ryan J Ollos

I haven't done any testing of the patch yet, but from reviewing it, I have the following comments:

  • For future use in other contexts, the function name could be more general than show_files_as_int. For example, item_count_str_to_int, which isn't a great name, but the best I could come up with off-hand. It should probably go in trac.util.__init__, but someone else might have a better idea.
  • I think we don't need to pass message to get_message_with_files since we can just append the output of get_message_with_files to the message string.
  • It looks like the files that the user doesn't have permission to view are just skipped. Should we render them as non-followable links?
  • Before committing we'll want to add unit tests in branches/1.0-stable/tracopt/ticket/tests/commit_updater.py, and for the show_files_as_int function.

Jun, please don't hesitate to take ownership of the ticket if you'd like to run with it. I'm working a number of other tickets at the moment, so I'll keep busy regardless.

comment:9 in reply to:  8 Changed 5 years ago by anonymous

show_files_as_int

trac.util.as_int(show_files, 0) is almost the same, but doesn't support unlimited. Maybe that could be (optionally) added there, or handled separately, or dropped as it isn't documented anyway.

Changed 5 years ago by t2y <tetsuya.morimoto@…>

Changed 5 years ago by t2y <tetsuya.morimoto@…>

comment:10 in reply to:  8 Changed 5 years ago by t2y <tetsuya.morimoto@…>

Replying to rjollos:

Thank you for reviewing, rjollos.

  • For future use in other contexts, the function name could be more general than show_files_as_int. For example, item_count_str_to_int, which isn't a great name, but the best I could come up with off-hand. It should probably go in trac.util.__init__, but someone else might have a better idea.

Maybe yes, I ask jomae to fix good name and location.

  • I think we don't need to pass message to get_message_with_files since we can just append the output of get_message_with_files to the message string.

OK. I got it and improved.

  • It looks like the files that the user doesn't have permission to view are just skipped. Should we render them as non-followable links?

I'm not sure which is better. I follow Trac-team's decision.

Once more, I changed a link for the modified file since its duplicated file name is redundant in my opinion when I used actually.

The Screen shot is here.

comment:11 Changed 4 years ago by Ryan J Ollos

Milestone: 1.1.3next-dev-1.1.x
Owner: Ryan J Ollos deleted
Status: assignednew

comment:12 Changed 4 years ago by Ryan J Ollos

Milestone: next-dev-1.1.x1.1.3
Owner: set to Ryan J Ollos
Status: newassigned

comment:13 Changed 4 years ago by Ryan J Ollos

Milestone: 1.1.31.1.4

comment:14 Changed 4 years ago by Ryan J Ollos

When [timeline] changeset_show_files is -1 or some integer, the following view is shown:

Consider just reusing the code that prepare this markup in the CommitTicketReferenceMacro. There are some nice points about this:

The last point could be considered a downside. However, having too many links makes the content on the page unpleasant. Navigating to a file would take only one extra click: click on changeset first, then on file.

I'm interest to hear thoughts on this.

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

Changed 4 years ago by Ryan J Ollos

Attachment: Timeline.png added

comment:15 in reply to:  14 Changed 4 years ago by Peter Suter

Replying to rjollos:

Consider just reusing the code that prepare this markup in the CommitTicketReferenceMacro. There are some nice points about this:

The last point could be considered a downside. However, having too many links makes the content on the page difficult to view. Navigating to a file would take only one extra click: click on changeset first, then on file.

+1 for reusing existing, robust code. I also like the status boxes and smaller font-size.

I imagine the missing links defeat the purpose a bit. But maybe we could linkify them (even in the timeline?) without making them more obtrusive? (E.g. similar to the links in the Last edited X minutes ago by Y (previous) (diff) hint.)

comment:16 in reply to:  14 Changed 4 years ago by t2y <tetsuya.morimoto@…>

Replying to rjollos:

  • There are no links to the changeset files to clutter the presentation.

Originally, I didn't expect to use this feature for timeline, so I don't need the links on there. I think the missing links is good for visibility.

comment:17 Changed 4 years ago by Ryan J Ollos

Milestone: 1.1.41.1.5

Narrowing focus for 1.1.4.

comment:18 Changed 3 years ago by Ryan J Ollos

Milestone: 1.1.51.2

comment:19 Changed 3 years ago by Ryan J Ollos

Milestone: 1.21.1.6

Milestone renamed

comment:20 Changed 3 years ago by Ryan J Ollos

Milestone: 1.1.6next-dev-1.1.x
Owner: Ryan J Ollos deleted
Status: assignednew

comment:21 Changed 3 years ago by Ryan J Ollos

Milestone: next-dev-1.1.xnext-dev-1.3.x

Narrowing focus for milestone:1.2. Please move ticket to milestone:1.2 if you intend to fix it.

comment:22 Changed 3 years ago by arkinform@…

Cc: arkinform@… added

comment:23 Changed 13 months ago by Ryan J Ollos

Milestone: next-dev-1.3.xnext-major-releases

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned.
as The resolution will be set.
The owner will be changed from (none) to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.