On Windows, we used to install the manual as Manual.html into the directory 'doc' in the installation prefix.
Maybe we could move the MINGW specific overrides in configure.ac to m4/geany-mingw.m4? You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/928
-- Commit Summary --
* Fix installation of the manual on Windows
-- File Changes --
M configure.ac (4) M doc/Makefile.am (17)
-- Patch Links --
https://github.com/geany/geany/pull/928.patch https://github.com/geany/geany/pull/928.diff
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928
Do we need to keep the paths different?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928#issuecomment-191758973
AFAICT reading the code, the only change actually needed is installing the index as `Manual.html` so the code finds it, but it searches in whatever `app->docdir` is set to in any case. And even there, we could easily make it check for `index.html` just like it does everwyehere else but on Windows, would that be a problem?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928#issuecomment-191778776
```diff diff --git a/geany.nsi.in b/geany.nsi.in index 9f55362..3053e13 100644 --- a/geany.nsi.in +++ b/geany.nsi.in @@ -191,7 +191,7 @@ Section "Documentation" SEC04 SetOverwrite ifnewer SetOutPath "$INSTDIR" File /r "${RESOURCEDIR}\doc" - WriteIniStr "$INSTDIR\Documentation.url" "InternetShortcut" "URL" "$INSTDIR\doc\Manual.html" + WriteIniStr "$INSTDIR\Documentation.url" "InternetShortcut" "URL" "$INSTDIR\doc\index.html" !insertmacro MUI_STARTMENU_WRITE_BEGIN ${PRODUCT_NAME} CreateShortCut "$SMPROGRAMS$StartmenuFolder\Documentation.lnk" "$INSTDIR\Documentation.url" !insertmacro MUI_STARTMENU_WRITE_END diff --git a/src/utils.c b/src/utils.c index a4372e4..23cb9af 100644 --- a/src/utils.c +++ b/src/utils.c @@ -1912,7 +1912,7 @@ gchar *utils_get_help_url(const gchar *suffix)
#ifdef G_OS_WIN32 skip = 8; - uri = g_strconcat("file:///", app->docdir, "/Manual.html", NULL); + uri = g_strconcat("file:///", app->docdir, "/index.html", NULL); g_strdelimit(uri, "\", '/'); /* replace '\' by '/' */ #else skip = 7; diff --git a/wscript b/wscript index d7cabeb..fa10df9 100644 --- a/wscript +++ b/wscript @@ -686,7 +686,7 @@ def build(bld): local_html_doc_filename = os.path.join(bld.path.abspath(), 'doc', 'geany.html') if os.path.exists(html_doc_filename) or os.path.exists(local_html_doc_filename): html_dir = '' if is_win32 else 'html/' - html_name = 'Manual.html' if is_win32 else 'index.html' + html_name = 'index.html' start_dir = bld.path.find_dir('doc/images') bld.install_files('${DOCDIR}/%simages' % html_dir, start_dir.ant_glob('*.png'), cwd=start_dir) bld.install_as('${DOCDIR}/%s%s' % (html_dir, html_name), 'doc/geany.html') ```
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928#issuecomment-191780135
@@ -120,6 +120,10 @@ GEANY_CHECK_THE_FORCE dnl hehe # i18n GEANY_I18N
+AM_COND_IF([MINGW],
- [docdir='${prefix}/doc'],
- [docdir='${docdir}'])
As the Travis failure shows, this is a recursive variable from Make's POV, not a good thing :) Could probably drop the else part and leave it as default, couldn't it?
This said, I think it'd probably be better to alter paths like this in the Makefile.am instead.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928/files#r54884739
BTW there are 2 other references to *index.html*, in `libmain.c:create_config_dir()`, which have no Windows specific variants.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928#issuecomment-191782051
@@ -120,6 +120,10 @@ GEANY_CHECK_THE_FORCE dnl hehe # i18n GEANY_I18N
+AM_COND_IF([MINGW],
- [docdir='${prefix}/doc'],
- [docdir='${docdir}'])
This said, I think it'd probably be better to alter paths like this in the Makefile.am instead.
Oh, I see, it's used in src/Makefile.am too. Well, err hum… so well yeah either not special-case Windows, or indeed to it at configure.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928/files#r54887931
https://github.com/b4n/geany/commit/40a252747292780a585e32a3cc014d65895aab25 (use same doc path on Windows as other platforms) not tested at all, but would something like this not be better for everyone? :)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928#issuecomment-191815546
@@ -120,6 +120,10 @@ GEANY_CHECK_THE_FORCE dnl hehe # i18n GEANY_I18N
+AM_COND_IF([MINGW],
- [docdir='${prefix}/doc'],
- [docdir='${docdir}'])
+AM_COND_IF([MINGW],
- [docdir='${prefix}/doc'],
- [docdir='${docdir}'])
As the Travis failure shows, this is a recursive variable from Make's POV, not a good thing :)
Oops. I really know too less about Autotools and how it works. Sorry.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928/files#r54989969
I'd be fine with https://github.com/b4n/geany/commit/40a252747292780a585e32a3cc014d65895aab25. My previous attempt was motivated by making as few changes to the existing Windows layout as necessary. However, changing the doc path to be the same as on non-Windows, seems way cleaner and removes even special code. So yes, let's go for it.
Rationale behind the original reason for having "doc" on Windows on the top-level installation directory: I'd be happy if I could remember exactly :). Most probably I made this to let users find the documentation instantly by checking the installation directory. Today I think this is not so necessary as users can easily open the documentation from within Geany using Help->Help and we also even put a clickable link to the documentation in the installation directory. These two things should be easy enough for the user, actually.
@b4n would you like to commit https://github.com/b4n/geany/commit/40a252747292780a585e32a3cc014d65895aab25 directly? I just tested it and docs are properly installed on Windows and can be opened from within Geany. I didn't test the NSIS changes yet but will do this as my next step is creating and testing the installer. Once the docdir change is committed in Geany, I'll update https://github.com/geany/geany-plugins/pull/376 to make this change also for G-P.
Thanks for your input and work!
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928#issuecomment-192107881
Good to see Enrico making the traditional "just before release" windows changes ready for 1.27.1 ;)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928#issuecomment-192158261
Good to see Enrico making the traditional "just before release" windows changes ready for 1.27.1 ;)
yeah, business as usual. I hoped we can avoid it this time but there was long time no progress after the 'let's kill Waf' hype. And now it's too late to wait for anyone else stepping up to fix the Windows stuff :(.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928#issuecomment-192171142
Closed #928 via f4b270a60e1d825de2dcc4db9951463fe32d5914.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928#event-578254067
@eht16 done, with a slight modification to the NSIS installer because I believe there was an obvious issue installing the new files. Anyway, if you'll try and fix it later it should be alright even if it's still not so perfect, right :)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928#issuecomment-192430360
@b4n thanks. I had to further tweak the doc installation a bit as done in https://github.com/geany/geany/commit/c9a799bb0c6a69a43ac690cbd78c9c464a1c64.... Now the docs are properly installed and also work from within Geany. Just tested.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928#issuecomment-192919515
@eht16 good! I see the problem, I didn't figure at the time that $RESOURCESDIR was a tar layout.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/928#issuecomment-192919605
github-comments@lists.geany.org