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

Updates for 3.0 #2258

Merged
merged 10 commits into from
Feb 17, 2018
Merged

Updates for 3.0 #2258

merged 10 commits into from
Feb 17, 2018

Conversation

anitagraser
Copy link
Member

@anitagraser anitagraser commented Dec 10, 2017

as well as some fixes for consistency reasons and some details that have been missing so far.

Needs a new screenshot for "Creating a new Shapefile layer dialog" that shows the "Geometries with Z coordinate" checkbox.
Fix #1937
ref #2252 (miss a screenshot)

as well as some fixes for consistency reasons and some details that have been missing so far.

Needs a new screenshot for "Creating a new Shapefile layer dialog" that shows the "Geometries with Z coordinate" checkbox.
Copy link
Member

@yjacolin yjacolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one typo to change. Thanks Anita!

Empty, editable memory layers can be defined using :menuselection:`Layer -->
Create Layer --> New Temporary Scratch Layer`. Here you can even create
Empty, editable temporary scratch layers can be defined using :menuselection:`Layer -->
Create Layer --> New Temporary Scratch Layer`. Here you can create
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you can create Too much space before create

@@ -319,8 +319,8 @@ The :guilabel:`DXF Export` dialog allows the user to:

.. _paste_into_layer:

Create layer from a clipboard
=============================
Createing new layers from the clipboard
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creat*e*ing -> Creating

Copy link
Collaborator

@DelazJ DelazJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @anitagraser This is why we need more reviewers particularly when big parts of doc are restructured. It really looks better and consistent.
I would be interested in backporting this commit to 2.18 (because text looks better but also for URL consistency) and, except Z dimension, there's no incompatibility yet . Any objection?

|selectString| and :guilabel:`Date` |selectString| attributes are
supported. Additionally, depending on the attribute type, you can also define
the length and precision of the new attribute column. Once you are happy with the
attributes, click **[OK]** and provide a name for the Shapefile. QGIS will
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has changed: in the new dialog the user fills the shapefile's name at the beginning, not after he clicks ok.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I see the changes in master now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the instructions.

saved and will be discarded when QGIS is closed. See also :ref:`paste_into_layer`.
|radioButtonOff|:guilabel:`Polygon` layers. Temporary Scratch Layers only exist in
memory. This means that they are not saved and will be discarded when QGIS is closed.
See also :ref:`paste_into_layer`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if what temporary scratch layer is (and how it lives or dies) shouldn't be exposed before steps to create it… ie move these last sentences to the beginning

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

|radioButtonOff|:guilabel:`Multipoint`, |radioButtonOff|:guilabel:`Multiline`
and |radioButtonOff|:guilabel:`Multipolygon` Layers beneath
|radioButtonOn|:guilabel:`Point`, |radioButtonOff|:guilabel:`Line` and
|radioButtonOff|:guilabel:`Polygon` Layers. Temporary Scratch Layers are not
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment actually concerns the not changed lines above: the spelling of geometry type has changed in recent commits, eg MultiPolygon instead of Multipolygon.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to say that it's no longer radio buttons but all in a combobox

Joins and complex queries can also be created simply by directly using the
names of the layers that are to be joined.
Joins and complex queries can also be created, for example, to join airports
and county information:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

county or country? (see the sample code)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, should be country, but doesn't really matter.

@anitagraser
Copy link
Member Author

@DelazJ Thanks and sure feel free to backport any part that's appropriate for 2.18.

@anitagraser
Copy link
Member Author

As far as I can see, all requested changes have now been addressed. Thank you for reviewing!

Copy link
Collaborator

@DelazJ DelazJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more comments



.. index:: Save layer
.. _general_saveas:

Save layer from an existing file
================================
Creating new layers from an existing layers
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An existing layer

@@ -163,35 +163,38 @@ predefined structure.
Creating a new Temporary Scratch Layer
--------------------------------------

Empty, editable memory layers can be defined using :menuselection:`Layer -->
Create Layer --> New Temporary Scratch Layer`. Here you can even create
Temporary Scratch Layers only exist in memory. This means that they are not saved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anitagraser could you please review #2260? I included some changes in this section that i forgot to propose here. It'd be nice to harmonize this description to avoid double work for translation team.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've integrated your changes here.

The first step is to provide a path and name for the Shapefile. QGIS will
automatically add the :file:`.shp` extension to the name you specify
Next, choose the type of layer (point, line or polygon) and optional Z or M
dimensions, as well as the CRS (coordinate reference system).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be Coordinate Reference System (ie titled)?

Copy link
Member Author

@anitagraser anitagraser Dec 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you usually write it in title case? It's not a name, so I'd usually keep it in lower case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! Never paid attention to it. I can see both spelling but i think you are not wrong writing it in lower case

@DelazJ
Copy link
Collaborator

DelazJ commented Dec 17, 2017

First of all, let me stress again that i approve changes in this PR and agree that they add consistency, hence improvements. This is the way we need to go.
BUT
I'd also like to raise some issues it will create once merged. I've already expressed this concern many times ago (eg qgis/QGIS#4023 (comment) or in this compilation ) but will do it again, for the last time: because QGIS HELP system is based on full url, merging this pull request will break links from the dxf export, save vector/raster as, embedded layers dialogs (and so will do some of the other PRs Anita opened - Thanks and keep on fixing those headings and formulation, please). In the past, i silently fixed the url in QGIS repo when the changes in docs occured by my fault but I'm not sure that:

  • QGIS Help system should rely on the memory of a single person to not bring readers to a wrong place;
  • QGIS Doc team should have to chase wrong links in the application
  • QGIS Doc team should worry about breaking url in the application, hence avoid changes that would improve the user manual (in this pr, headings are simply renamed so user will fallback to the chapter level but in some cases we can need to move sections to another chapter - see eg Refactoring the Supported data formats chapter #1996 for the silent fix i mentioned beforehand - and with QGIS 3 changes, this could likely occur again when features are documented).

So, what to do? I can see two options (there might be others):

  • use rst internal anchors (the one we use to cross-link sections) which likely do not have any reason to change. I don't know how easy the implementation could be nor whether it's desirable or not to build help infrastructure close to rst language but this guarantee that help urls are always uptodate, regardless the heading and the chapter.
  • report missing links in application so that devs fix them. @jgrocha wrote a script to list unreached links (see it in action here); it could be improved if needed, and show warning (or whatever better suits) when a link is not found. Anybody willing to fix it can then do that.

Voilà my last call on what looks like an issue to me but i could be wrong. A lot has already been done; it'd be a pity to lose those missing bits. And sorry for the length (i wanted it as documented as possible - aren't we in Docs repo?).
Cc @m-kuhn @alexbruy @rduivenvoorde @SrNetoChan @ghtmtt ...

@anitagraser
Copy link
Member Author

Thank you for the explanations @DelazJ! I wasn't aware that the QGIS help system relies on headings rather than anchors. Sound to me like anchors are the way to go.

@DelazJ
Copy link
Collaborator

DelazJ commented Dec 18, 2017

@rduivenvoorde anything wrong with the build? The latest commit shouldn't have failed.

@rduivenvoorde
Copy link
Contributor

@DelazJ all is ok now isn't it? Travis had troubles receiving Python apparently (nothing to do with the commits). But next build was ok.

@ghtmtt
Copy link
Contributor

ghtmtt commented Dec 19, 2017

@DelazJ IMHO rst anchors are the way to follow. Also in the Processing Help files refactoring all the algorithms have an unique anchor (that will remain unique also in the future).
I think @alexbruy knows the code perfectly. What is your opinion?

@rduivenvoorde
Copy link
Contributor

@m-kuhn @alexbruy @rduivenvoorde @SrNetoChan @ghtmtt @wonder-sk

About the searching for any doc url's in the QGIS source files:

Martin was proposing a long time ago that we should have an (ini?) file containing 'key' to 'url's.
So in the source there will be a reference ONLY to the key.
There has to be some 'find_url_for_key' method then off course too...

This indirection would make it easier to:

  • check if urls are stil valid/reachable
  • grep sources to see if all keys are still used

Is this something to consider?

@m-kuhn
Copy link
Member

m-kuhn commented Dec 19, 2017

@rduivenvoorde Could we also grep the sources for QgsHelp::openHelp and do the check on that? I think that should have the same result and there won't be the requirement to maintain a separate file additionally.

@DelazJ
Copy link
Collaborator

DelazJ commented Dec 19, 2017

Could we also grep the sources for QgsHelp::openHelp and do the check on that?

@m-kuhn isn't it what is done in the aforementioned script from @jgrocha ? (unless I misunderstand)

@m-kuhn
Copy link
Member

m-kuhn commented Dec 19, 2017

Oh, yes, pretty much :)

@rduivenvoorde
Copy link
Contributor

@m-kuhn yep, except that the ini file could be maintained and controlled by documentation team:
moving pages around in the doc tree, adding another 'anchor'...
Keys and Url would be in one place instead of scattered over the sources.
I was hoping that in this file would 'merge' other help systems we have...
Grepping is not for a normal task for average Windows user...

@m-kuhn
Copy link
Member

m-kuhn commented Dec 19, 2017

I see. Why not doing that on the server? Less deploy problems. And easy to do with mod rewrite etc.

@rduivenvoorde
Copy link
Contributor

@m-kuhn It is not easy to do. I'll show you our current server configs if you want. Please do not force more rewrites, redirects, etc etc etc etc
But happy to hand over that responsibility over these though, if it is so 'easy'!

Else forget the idea. If at least we check the url's in feature freeze so for every release they are all valid.

@m-kuhn
Copy link
Member

m-kuhn commented Dec 20, 2017

Sorry for claiming that it's easy, I admit I don't know about the server setup and I'm sure that everything behind is not a simple task. You are doing a great job with maintaining this.
I would still appreciate if we could discuss a mapping on the server. From what I can see this will be more flexible and maintainable than one on clients. If it is shipped with the binaries, each change (and changes are expected or we wouldn't put this system in place at all) needs to go through an update, and unfortunately not everyone is always on the latest point release.

@anitagraser
Copy link
Member Author

From a documentation writer's perspective, it would feel more empowering if changes in the documentation structure could be made without bothering with the actual software sources.
Also strong +1 for Richard's "Grepping is not for a normal task for average Windows user...". If we want to involve more people in writing documentation, most of the tasks should be possible by using Github's web interface and checking Travis messages.

@SrNetoChan
Copy link
Member

SrNetoChan commented Jan 2, 2018

About getting more people writing documentation, I have been preparing a PR to make the figures show in github rst preview.

#2301

I am hoping to have some discussion around the inline substituitions, please comment if you have any concerns or ideas.

Anyway, about the "Grepping", we need to remember that not everything can be done on github. There is no conflicts management, it's not possible to upload/edit new files. So we will always need to "grepping" guys that can manage that for others writers. I am ok with it.

@SrNetoChan
Copy link
Member

Regarding the helping system, even using the sphinx anchors I would like to explore the idea of the {key : value} ini file as a one place to maintain.

One of the problems that we may have is not having certain sections of the documentation ready by the time of QGIS release, therefore, we won't know what link anchor to use in the application side. @alexbruy could the QGIS aplication rely on this key value file (editable on the documentation side) to redirect the user to the right section? This way we could update that file when needed.

@m-kuhn
Copy link
Member

m-kuhn commented Jan 3, 2018

From a documentation writer's perspective, it would feel more empowering if changes in the documentation structure could be made without bothering with the actual software sources. Also strong +1 for Richard's "Grepping is not for a normal task for average Windows user...". If we want to involve more people in writing documentation, most of the tasks should be possible by using Github's web interface and checking Travis messages.

I totally agree that doc writers should not bother with the app sources nor have to use grep or other command line tools anywhere.
I can assure you that the system I have in mind does not require any of these tasks.

@m-kuhn
Copy link
Member

m-kuhn commented Jan 3, 2018

Following up on this, none of the approaches outlined in here would require such a manual step.

In general I think the following system should allow for minimal manual effort while having a good overview of the status:

  1. write the doc

  2. create a link in the source code

  3. have a system that regularly (e.g. nightly) checks if the links in the code point to real targets (something like the aforementioned script from @jgrocha)

  4. this script updates a table in the web with 3 states

    • documentation ok
    • documentation missing (not yet done)
    • documentation did exist but no longer does exist (redirect required)

@DelazJ
Copy link
Collaborator

DelazJ commented Jan 16, 2018

@m-kuhn @rduivenvoorde thanks for this brainstorming. What/when would then be the next step?

@DelazJ
Copy link
Collaborator

DelazJ commented Feb 17, 2018

Let's move ahead and clean pr queue

@DelazJ DelazJ merged commit 47c7feb into master Feb 17, 2018
@DelazJ DelazJ deleted the anitagraser-patch-1 branch February 17, 2018 05:11
@DelazJ
Copy link
Collaborator

DelazJ commented Feb 17, 2018

Thanks Anita

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

Successfully merging this pull request may close these issues.

Creating a new shapefile allows Z coordinate geometries
7 participants