Modify

Opened 4 years ago

Closed 4 months ago

#12729 closed enhancement (fixed)

Only Include a Section of a Source File

Reported by: zappotek Owned by: ttressieres
Priority: normal Component: IncludeMacro
Severity: normal Keywords: include slice source file
Cc: Massimo Trac Release:

Description

For including source files from the repository it would really nice to have finer control over what is included. This is really handy for referring to problematic code sections within a ticket.

The patch of #9537 might be a start but it doesn't work with source includes.

The attached patch allows to specify a range (basically a python slice) of lines to include. It uses the ? (maybe not the best choice?) to separate the file name from the slice arguments:
[[Include(source:<file_name>?<start>:<stop>:<step>)

For example, the lines 10 to 19 of file main.f90 by

[[Include(source:my_source.f90?10:20, text/x-fortran)]]

Attachments (5)

includeMacro_srcRange.diff (1.0 KB) - added by zappotek <webseppl@…> 4 years ago.
handle_partial_source_file.patch (6.3 KB) - added by ttressieres 4 months ago.
handle partial display of source files
handle_partial_source_file-2.patch (6.8 KB) - added by ttressieres 4 months ago.
second version with named parameters
handle_partial_source_file-3.patch (6.4 KB) - added by ttressieres 4 months ago.
new version with use of as_bool & as_int
handle_partial_source_file-4.patch (6.6 KB) - added by ttressieres 4 months ago.
new version with exceptions and strip parameters

Download all attachments as: .zip

Change History (26)

Changed 4 years ago by zappotek <webseppl@…>

Attachment: includeMacro_srcRange.diff added

comment:1 Changed 4 years ago by Ryan J Ollos

Cc: Massimo added

This is a good idea, as it would allow us to deprecate IncludeSourcePartialPlugin.

comment:2 in reply to:  1 ; Changed 4 months ago by Ryan J Ollos

Cc: ttressieres added

Replying to Ryan J Ollos:

This is a good idea, as it would allow us to deprecate IncludeSourcePartialPlugin.

Or maybe IncludeSourcePartialPlugin can be merged into IncludeMacro. What do you think @ttressieres?

comment:3 in reply to:  2 Changed 4 months ago by ttressieres

Replying to Ryan J Ollos:

Replying to Ryan J Ollos:

This is a good idea, as it would allow us to deprecate IncludeSourcePartialPlugin.

Or maybe IncludeSourcePartialPlugin can be merged into IncludeMacro. What do you think @ttressieres?

Yes it could be a good idea, I just created a patch to merge the code from IncludeSourcePartialPlugin in this macro, it's just a draft...

The macro will take an extra parameter when 'source' is used the syntax is 'start:end:lineno'

  • 'start' and 'end' could be a number or a regex
  • 'lineno' to specify if we want to display line numbers
Last edited 4 months ago by Ryan J Ollos (previous) (diff)

Changed 4 months ago by ttressieres

handle partial display of source files

comment:4 Changed 4 months ago by Ryan J Ollos

Status: newaccepted

Thanks! I'll commit the patch and give you commit access for this plugin.

comment:5 Changed 4 months ago by Ryan J Ollos

Initial feedback:

  • The N:M syntax is nice, but we could also support the start and end keywords like IncludeSourcePartialPlugin#Description. And/or, I think we should consider using TracLinks syntax like @200:27-30, where 200 is the rev and 27-30 are the lines. We already support a peg revision like @200.
  • For line numbering, I definitely prefer a keyword: lineno=True.
  • Are you planning to continue developing IncludeSourcePartialPlugin? Or finalize that code and switch over to IncludeMacro development? I've granted you commit access to IncludeMacro.

Thoughts?

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

comment:6 Changed 4 months ago by ttressieres

  • I was not very satisfied with my syntax "start:end:line", I will look at your different options.

Regards

Changed 4 months ago by ttressieres

second version with named parameters

comment:7 Changed 4 months ago by ttressieres

See the new patch with named parameters : start, end, lineno

It's much cleaner, if you're agree I will commit it and close this ticket

comment:8 in reply to:  6 ; Changed 4 months ago by Ryan J Ollos

Replying to ttressieres:

Edited IncludeSourcePartialPlugin@11. Should we reassign the two open tickets (#5233 and #6172) to this plugin?

comment:9 in reply to:  7 Changed 4 months ago by Ryan J Ollos

Replying to ttressieres:

It's much cleaner, if you're agree I will commit it and close this ticket

Thanks, I will review it today.

comment:10 in reply to:  8 Changed 4 months ago by ttressieres

Replying to Ryan J Ollos:

Replying to ttressieres:

Edited IncludeSourcePartialPlugin@11. Should we reassign the two open tickets (#5233 and #6172) to this plugin?

OK, I fixed the #5233 and reassign #6172 to IncludeMacro

comment:11 Changed 4 months ago by Ryan J Ollos

as_bool (from trac.util import as_bool) should be used for:

                    try:
                        lineno = int(lineno)
                    except:
                        negatory = ('no', 'false', 'none')
                        lineno = str(lineno).lower() not in negatory

lineno should probably default to False.

lineno = as_bool(kwargs.get('lineno')) should accomplish both of those aims.

dict.get returns None for missing keys, so end = kwargs.get('end', None) -> end = kwargs.get('end').

Minor thing, I'd extract kwargs in expand_macro and pass start, end, lineno to the private method.

I have a few more comments shortly. I'm going to remove some obsolete code from macros.py.

comment:12 Changed 4 months ago by Ryan J Ollos

In 17698:

TracIncludeMacro 3.2.0dev: Remove Trac 0.11 compatibility code

The macro supports Trac >= 1.0.

Refs #12729.

comment:13 Changed 4 months ago by Ryan J Ollos

You could also use as_int (from trac.util import as_int) in _handle_partial_source. as_int will trap exceptions should return default, so you can avoid the try / except.

And arguments should be separated by spaces per PEP8: src.count('\n',0,match.start()) -> src.count('\n', 0, match.start()).

I'll standby to test next revision of patch. Thanks!

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

comment:14 Changed 4 months ago by Ryan J Ollos

With some effort we could move towards making this code suitable for inclusion in Trac.

Changed 4 months ago by ttressieres

new version with use of as_bool & as_int

comment:15 Changed 4 months ago by ttressieres

I hope it matches all your advices :-)

comment:16 Changed 4 months ago by Ryan J Ollos

Thanks! Looking good.

Minor thing, the example end=15 lineno=yes is missing a comma: end=15, lineno=yes. And the comment for _handle_partial_source related to Subversion is probably no longer relevant since the Trac API is used rather than directly using the Subversion API.

Examples using Regex would be nice.

If the start regex doesn't match anything, I get a TypeError. If the end regex doesn't match then it defaults to the last line of the file. It would be good to return a system message saying the regex doesn't match.

14:35:03 Trac[formatter] ERROR: Macro Include(browser:trac/COPYING, start=^ABC, end=30) failed for <Resource u'wiki:IncludeMacro'>:
Traceback (most recent call last):
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/wiki/formatter.py", line 824, in _macro_formatter
    return macro.ensure_inline(macro.process(args), in_paragraph)
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/wiki/formatter.py", line 396, in process
    text = self.processor(text)
  File "/Users/rjollos/Documents/Workspace/trac-dev/teo-rjollos.git/trac/wiki/formatter.py", line 368, in _macro_processor
    text)
  File "/Users/rjollos/Documents/Workspace/trac-dev/trac-hacks/includemacro/trunk/includemacro/macros.py", line 147, in expand_macro
    dest_format, start, end, lineno)
  File "/Users/rjollos/Documents/Workspace/trac-dev/trac-hacks/includemacro/trunk/includemacro/macros.py", line 272, in _get_source
    out, start, end = self._handle_partial_source(out, start, end)
  File "/Users/rjollos/Documents/Workspace/trac-dev/trac-hacks/includemacro/trunk/includemacro/macros.py", line 244, in _handle_partial_source
    end += start
TypeError: unsupported operand type(s) for +=: 'int' and 'unicode'

I put my regex in quotes when testing, which doesn't work. We might want to strip single and double quotes when the argument is wrapped.

This works: [[Include(browser:trac/COPYING, start=5, end=^THIS SOFTWARE)]]

This doesn't: [[Include(browser:trac/COPYING, start=5, end="^THIS SOFTWARE")]]

comment:17 Changed 4 months ago by ttressieres

Yes, you're right, your advices are interesting.

I don't know if we want to strip quotes before parsing integer or not. Is "5" equals 5 or not ?

Thanks for your comments

Changed 4 months ago by ttressieres

new version with exceptions and strip parameters

comment:18 in reply to:  17 Changed 4 months ago by Ryan J Ollos

Replying to ttressieres:

I don't know if we want to strip quotes before parsing integer or not. Is "5" equals 5 or not ?

Good point, it could get ambiguous. That's the downside to accepting multiple data formats in a keyword parameter.

We should probably strip only after we determine that it's a string:

        start = as_int(start, start)
        if isinstance(start, basestring):
            start = start.strip("'\"")
            end = as_int(end, end)
            if isinstance(end, basestring):
                end = end.strip("'\"")

Other than that change, I think it's good. If you agree, feel free to go ahead and commit.

comment:19 Changed 4 months ago by Ryan J Ollos

Cc: ttressieres removed
Owner: changed from Ryan J Ollos to ttressieres
Status: acceptedassigned

comment:20 Changed 4 months ago by Ryan J Ollos

CiteCodeMacro is similar. We can probably deprecate that plugin.

See also CodeExampleMacro and trac:#13277.

comment:21 Changed 4 months ago by ttressieres

Resolution: fixed
Status: assignedclosed

In 17709:

include a section of source file, accept 3 named parameters "start", "end", "lineno" (close #12729)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain ttressieres.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.