issue-#140 Unwrapping IMPORT aliasses

Merged to the master branch

Re: issue-#140 Unwrapping IMPORT aliasses

Postby Robert » Mon Nov 14, 2016 8:59 am

If there are no more comments, maybe this topic is ready for a vote.
User avatar
Robert
 
Posts: 1023
Joined: Sat Sep 28, 2013 11:04 am
Location: Edinburgh, Scotland

Re: issue-#140 Unwrapping IMPORT aliasses

Postby DGDanforth » Mon Nov 14, 2016 9:06 am

Robert wrote:If there are no more comments, maybe this topic is ready for a vote.

I have a very naive comment having only cursory followed this discussion.
I have written in the past for myself a simple imports program that extracts the list of imports used by a module.
It skips directly to IMPORT without worrying about MODULE. It removes the alias from any import that may have one.
The only thing that it doesn't handle is comments in the import list.

So, I am not really sure what all the fuss is about Unwrapping IMPORT aliasses.

-Doug
User avatar
DGDanforth
 
Posts: 1061
Joined: Tue Sep 17, 2013 1:16 am
Location: Palo Alto, California, USA

Re: issue-#140 Unwrapping IMPORT aliasses

Postby Josef Templ » Mon Nov 14, 2016 10:05 am

Robert wrote:If there are no more comments, maybe this topic is ready for a vote.


One minor point in DevBrowser:

> DevReferences.ResolveImportAlias(s.string, t);

This line is colored. Since we have the changes in git's change log anyway,
we do not use colors to mark changes. The problem with using colors is that
after some time you are running out of colors, so this does not scale well.

- Josef
User avatar
Josef Templ
 
Posts: 2038
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#140 Unwrapping IMPORT aliasses

Postby Robert » Tue Nov 15, 2016 8:51 am

Josef Templ wrote:One minor point in DevBrowser: ...

I will post the corrected file this evening.

I noticed a couple of unrelated tidy-ups that might be considered at the same time:

- There are several old sections of commented out code. I would be tempted to delete these.
- There are one or two empty IF statements. I would be tempted to delete these also, or at least comment them out.
User avatar
Robert
 
Posts: 1023
Joined: Sat Sep 28, 2013 11:04 am
Location: Edinburgh, Scotland

Re: issue-#140 Unwrapping IMPORT aliasses

Postby Robert » Tue Nov 15, 2016 9:03 am

DGDanforth wrote:So, I am not really sure what all the fuss is about Unwrapping IMPORT aliasses.

I have possibly misunderstood your post ... .

You seem to be suggesting that simple ways of parsing the IMPORT list are adequate in practice, and so a discussion about elaborate parsing logic is unnecessary.

This topic is not about that. In truth, I have not even tried to understand what the BlackBox parsing logic is. The point is that BlackBox uses its logic for some built in (Menu) Commands, but not all. In this topic we simply apply the existing logic (what ever that is) to these other Commands (eg "Info -> Global Variables"). The BlackBox parsing logic has always been fine, so I saw no need to question it.

A side effect of this has been an overall simplification (by reuse) of the total code.
User avatar
Robert
 
Posts: 1023
Joined: Sat Sep 28, 2013 11:04 am
Location: Edinburgh, Scotland

Re: issue-#140 Unwrapping IMPORT aliasses

Postby Josef Templ » Tue Nov 15, 2016 1:41 pm

Robert wrote:I noticed a couple of unrelated tidy-ups that might be considered at the same time:

- There are several old sections of commented out code. I would be tempted to delete these.
- There are one or two empty IF statements. I would be tempted to delete these also, or at least comment them out.


I know that it is tempting to cleanup the code on such an occasion.
It is not the topic of this issue unless the code is in close vicinity to your other changes.
For 1.7 most of the time I have resisted the temptation.

When later looking at the changes associated with an issue
it is a big advantage if the changes are strictly related to the issue.
It is also easier to review the changes of an issue if the change list is short.
It is sometimes also not clear if it is an advantage to delete commented code or not
because it may have some documentation value that something has been removed.

- Josef
User avatar
Josef Templ
 
Posts: 2038
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#140 Unwrapping IMPORT aliasses

Postby DGDanforth » Tue Nov 15, 2016 7:43 pm

Robert wrote:A side effect of this has been an overall simplification (by reuse) of the total code.

OK, I went back to the beginnings of this discussion and now understand what you are doing.
Yes, it is good to have the code localized to one place and accessible to anyone.
-Doug
User avatar
DGDanforth
 
Posts: 1061
Joined: Tue Sep 17, 2013 1:16 am
Location: Palo Alto, California, USA

Re: issue-#140 Unwrapping IMPORT aliasses

Postby Robert » Tue Nov 15, 2016 7:45 pm

Updated version now available.

The latest diff is: http://redmine.blackboxframework.org/pr ... 31afd4a6a7

(The previous diff is rather more informative!).
User avatar
Robert
 
Posts: 1023
Joined: Sat Sep 28, 2013 11:04 am
Location: Edinburgh, Scotland

Re: issue-#140 Unwrapping IMPORT aliasses

Postby DGDanforth » Tue Nov 15, 2016 7:54 pm

Robert wrote:Updated version now available.

The latest diff is: http://redmine.blackboxframework.org/pr ... 31afd4a6a7

(The previous diff is rather more informative!).

Is that a joke?
No differences?
User avatar
DGDanforth
 
Posts: 1061
Joined: Tue Sep 17, 2013 1:16 am
Location: Palo Alto, California, USA

Re: issue-#140 Unwrapping IMPORT aliasses

Postby Robert » Tue Nov 15, 2016 9:43 pm

DGDanforth wrote:Is that a joke?
No differences?
No.
About 4 posts ago (14 Nov) Josef noticed that I had accidently left some color in a file, so I have set it back to defaultColor, and, as discussed just above, made no other changes.

Don't ask me how he noticed the color.
User avatar
Robert
 
Posts: 1023
Joined: Sat Sep 28, 2013 11:04 am
Location: Edinburgh, Scotland

PreviousNext

Return to Resolved (Features)

Who is online

Users browsing this forum: No registered users and 1 guest

cron