Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Locations starting with another one's name not navigable #2

Open
vwbij opened this issue Jan 17, 2017 · 6 comments
Open

Locations starting with another one's name not navigable #2

vwbij opened this issue Jan 17, 2017 · 6 comments
Assignees
Labels

Comments

@vwbij
Copy link

vwbij commented Jan 17, 2017

Hi,

The selection of the current repository entry in querySnapshot was broken. It finds a match for "KEY" in "KEY_AB", "KEY_DE" and so on. Here the fixed loop head:

while (loc)
{
    const int minLen = min(loc->title.len, pathLen);
	if (!strncmp(path, loc->title.data, minLen))
    {
        apr_pool_t *subPool = NULL;
        svn_error_t *err;
        svn_opt_revision_t revision;
        size_t subPathLen = 0;
        char *buf = NULL;
        strbuf_t s;

		// Lets look if this is correct. If the path is longer than the title, we need to see
		// a path seperator next (\ or /). If not, we have it to do with a matching part of a name
		// ("UDO_NEW" instead of "UDO"). Good match would be: "UDO/DevGuide" or "UDO\DevGuide".
		if(pathLen>minLen)
		{
			if(!((path[minLen]=='/')||(path[minLen]=='\\')))
			{
		        loc = loc->next;
				continue;
			}
		}

        /* found it, fetch data from SVN */
        subPool = svn_pool_create(Subversion.pool);
        subPathLen = strlen(path + minLen);
        buf = (char*)apr_palloc(subPool, loc->url.len + subPathLen + 1);
		s.data = buf;
		s.size = loc->url.len + subPathLen + 1;

All the Best,

Volker

@mvf
Copy link
Owner

mvf commented Feb 1, 2017

Hi Volker,

sorry for the late reply and thanks for taking the time to debug and report this. Unfortunately that part of the code seems broken on a conceptual level, so the proposed change is unlikely to fully fix it. At the very least there are other parts that rely on the same faulty reasoning. The root cause is understood and I'll look into it the next time I boot Windows.

Cheers,
Matthias

@mvf mvf self-assigned this Feb 1, 2017
@vwbij
Copy link
Author

vwbij commented Feb 2, 2017 via email

@mvf mvf added the bug label Feb 2, 2017
@mvf mvf changed the title Fixed Bug: selection in list broken Locations starting with another one's name not navigable Feb 2, 2017
@mvf
Copy link
Owner

mvf commented Feb 5, 2017

Hi Volker,

thanks for reporting all those points. I have taken the liberty of creating a separate issue for each one and I kindly ask that you do the same for any new topics that come up, regardless of whether it's a defect, request, suggestion, etc. This will make it much easier to keep track of things.

In general, patches are welcome of course, but I don't see any benefits in using C++. C99 is fine now though, since it's finally supported by Microsoft compilers.

Thanks,
Matthias

@vwbij
Copy link
Author

vwbij commented Feb 5, 2017 via email

@mvf
Copy link
Owner

mvf commented Feb 5, 2017

Hi Volker,

sorry if my C++ comment ended up sounding a bit hostile, I'm not opposed to C++ in general. I just don't see how it would help me personally in this somewhat low-level project. If you are more productive with C++, more power to you! 😉

One thing that's perhaps worth mentioning: ATM this program is even C89 because that's all Visual Studio supported at the time. Even now I remember that as extremely irritating. Not being able to mix declarations and code was the biggest issue because it degrades function structure. So on the off-chance you're using a version of Visual Studio older than Visual Studio 2013, upgrading would enable a useful subset of C99.

Happy hacking,
Matthias

@vwbij
Copy link
Author

vwbij commented Feb 7, 2017

Hi Matthias,

FYI / offtopic: I have finished a first version and send it to some fellows for test. You may download it from "https://dl.dropboxusercontent.com/u/15963916/TCP.ZIP" if you want. Source is not included yet, have to cleanup the code a little bit :-). Main improvement is the ability to get the "real" file size.

All the Best, Volker

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants