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

[t2y <tetsuya.morimoto@…>]

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

[Ryan J Ollos]

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.

[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.

[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?

[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​.

[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!

[t2y <tetsuya.morimoto@…>]

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

• attachment:show-changeset-files-in-comment-improved1.patch​​

The Screen shot is here.

[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.

[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.

[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.

[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 status boxes are used to indicate the change status.
• The font-size is slightly smaller, which would de-emphasize this content in the ticket comment.
• We can reuse the code that we already know to be robust: tags/trac-1.0.2/trac/versioncontrol/web_ui/changeset.py@:963-977#L939​.
• There are no links to the changeset files to clutter the presentation.

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.

[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 status boxes are used to indicate the change status.
• The font-size is slightly smaller, which would de-emphasize this content in the ticket comment.
• We can reuse the code that we already know to be robust: tags/trac-1.0.2/trac/versioncontrol/web_ui/changeset.py@:963-977#L939​.
• There are no links to the changeset files to clutter the presentation.

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.)

[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.

[Ryan J Ollos]

Narrowing focus for 1.1.4.

[Ryan J Ollos]

Milestone renamed

[Ryan J Ollos]

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